Skip to content

fix(policy): gate eval error fails-closed by default (PILOT-262)#6

Open
matthew-pilot wants to merge 1 commit into
mainfrom
openclaw/pilot-262-20260529-234710
Open

fix(policy): gate eval error fails-closed by default (PILOT-262)#6
matthew-pilot wants to merge 1 commit into
mainfrom
openclaw/pilot-262-20260529-234710

Conversation

@matthew-pilot
Copy link
Copy Markdown
Collaborator

What failed

Policy gate events (connect, dial, datagram) in runner.go:EvaluateGate returned true (ALLOW) when expression evaluation threw an error — legacy fail-open behaviour. A corrupted policy file, context drift, or expression timeout would silently widen into "allow everything."

EvaluateActions and evaluatePerPeerCycle similarly swallowed eval errors with a warn-level log only.

Root cause

EvaluateGate line 219: return true // fail open on error — no configurable override to flip to fail-closed.

Fix

Add FailClosed *bool to PolicyDocument (fail_closed in JSON), defaulting to true (fail-closed — more secure).

Location Change
policylang/policy.go Add FailClosed field with docs
policylang/engine.go Add FailClosed() bool helper (nil → true)
runner.go:EvaluateGate On error: if fail_closed → deny + publish policy.eval_error; else → log warn + allow
runner.go:EvaluateActions On error: if fail_closed → log error + publish event; else → log warn
runner.go:evaluatePerPeerCycle On error: if fail_closed → log error; else → silent
zz_audit_defensive_test.go Pin new default (fail-closed deny) + explicit fail_open legacy path

Operators who need legacy fail-open set "fail_closed": false in the policy JSON.

Verification

go build ./...   ✅
go vet ./...     ✅
go test ./...    ✅ (policy: 5.665s, policylang: 0.007s)
git diff --stat HEAD~1:
 policylang/engine.go       | 11 ++++++++
 policylang/policy.go       |  6 +++++
 runner.go                  | 27 ++++++++++++++++++--
 zz_audit_defensive_test.go | 70 ++++++++++++++++++++++++++++++++++++---------------
 4 files changed, 90 insertions(+), 24 deletions(-)

Closes PILOT-262

Policy gate events (connect, dial, datagram) returned ALLOW when
expression evaluation failed — fail-open behaviour that a corrupted
policy file or unexpected context drift could silently widen into
"allow everything."

Add a FailClosed field to PolicyDocument (fail_closed in JSON),
defaulting to true. When fail_closed:
- EvaluateGate returns DENY on eval error instead of ALLOW
- EvaluateActions / evaluatePerPeerCycle publish policy.eval_error
  events and log at ERROR instead of silently returning
- Operators who need legacy fail-open set fail_closed: false

Pin the new default in audit tests; add explicit fail_open pathway
test so the legacy behaviour stays covered.

Closes PILOT-262
@codecov
Copy link
Copy Markdown

codecov Bot commented May 29, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@matthew-pilot
Copy link
Copy Markdown
Collaborator Author

🦾 Matthew PR Check — #6 PILOT-262

Status

  • State: OPEN · MERGEABLE ✅
  • CI: 2/2 passing (test ✅, codecov/patch ✅)
  • Canary: not-configured
  • Files: 4 (+90/−24) — policylang/engine.go, policylang/policy.go, runner.go, zz_audit_defensive_test.go

Verdict

CLEAN — all CI green, mergeable, well-structured change with pinned tests.

What changed

Adds FailClosed *bool to PolicyDocument (JSON: fail_closed), defaulting to true (fail-closed — more secure). Expression evaluation errors in EvaluateGate, EvaluateActions, and evaluatePerPeerCycle now deny/skip when fail-closed, and publish policy.eval_error events. Operators wanting legacy fail-open set "fail_closed": false.

🔗 PILOT-262

@matthew-pilot
Copy link
Copy Markdown
Collaborator Author

🦾 Matthew Explains — #6 PILOT-262

What this does

Switches policy expression evaluation errors from fail-open (legacy: error → ALLOW) to fail-closed (new default: error → DENY). Adds a configurable fail_closed field to PolicyDocument.

Why

A corrupted policy file, context drift, or expression timeout previously widened into "allow everything" silently. The legacy behaviour was a documented trade-off in runner.go line 219: return true // fail open on error. PILOT-262 flips the default to fail-closed — more secure.

How it works

Site Fail-Closed Fail-Open (legacy, opt-in)
EvaluateGate DENY + publish policy.eval_error event slog.Warn + ALLOW
EvaluateActions slog.Error + skip directives + publish slog.Warn + skip
evaluatePerPeerCycle slog.Error + skip silent skip

Default when FailClosed is nil → true (fail-closed). Set "fail_closed": false in policy JSON for legacy behaviour.

Risk assessment

Low. The change is conservative:

  • Default fail-closed is the safer posture
  • Explicit opt-in path preserved for operators needing legacy fail-open
  • Pinned tests cover both paths: TestPin_EvaluateGate_FailClosedOnEvalError (new default) and TestPin_EvaluateGate_FailOpenExplicit (legacy)
  • CI all green (2/2)

@matthew-pilot
Copy link
Copy Markdown
Collaborator Author

📊 Status — PILOT-262

  • PR state: Open · Mergeable · No conflicts · No reviews yet
  • Canary: Not run — no canary job found for this PR
  • Jira: QA/IN-REVIEW — "policy: gate events return ALLOW on evaluation error (fail-open)" · Assigned: Teodor Calin · Last updated 2026-05-30
  • Operator activity: None — no comments/reviews from @TeoSlayer on this PR yet

🤖 auto-status by matthew-pilot

@matthew-pilot
Copy link
Copy Markdown
Collaborator Author

🤖 PR Clarification — Status Update

#6: fix(policy): gate eval error fails-closed by default (PILOT-262)
Status: open | Mergeable: yes (clean) ✅

Summary

Switches policy expression evaluation errors from fail-open (legacy: error → ALLOW) to fail-closed (new default: error → DENY). Adds configurable fail_closed field to PolicyDocument.

Changes

+90/−24 across 4 files: policylang/engine.go, policylang/policy.go, runner.go, zz_audit_defensive_test.go

CI Status

test ✅ | codecov/patch ✅ (2/2 green)

Labels

(none)


👋 @TeoSlayer — this PR is ready for review. No merge conflicts, all CI green.

matthew-pr-worker • 2026-06-01T20:47:00Z

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant