Skip to content

fix(policy): bind the resolved safety policy to dispatched runs - #76

Merged
abrichr merged 1 commit into
mainfrom
fix/bind-safety-policy-to-runs
Jul 27, 2026
Merged

fix(policy): bind the resolved safety policy to dispatched runs#76
abrichr merged 1 commit into
mainfrom
fix/bind-safety-policy-to-runs

Conversation

@abrichr

@abrichr abrichr commented Jul 27, 2026

Copy link
Copy Markdown
Member

The defect: a governance control that rendered but did not bind

The unified settings/policy system shipped in three parts. Parts 1 and 2 landed
(Cloud API + migration + dashboard; engine/policy.py's fetch/cache/harden
resolver). Part 3 — carrying the resolved policy into execution — never did.

Verified against origin/main refs only (no working tree):

$ git grep -n "policy" origin/main -- 'engine/*.py' | grep -E "import|from"
origin/main:engine/dispatch.py:2194:        from engine import policy as policy_mod
origin/main:engine/dispatch.py:2218:        from engine import policy as policy_mod
origin/main:engine/qualification.py:46:  from openadapt_flow.policy import evaluate_policy, ...

engine/policy.py's only callers on main are the two UI verbs in
dispatch.py (get_effective_policy, refresh_policy). engine/runner_loop.py
— the module that actually executes a dispatched governed run — contains zero
references to policy, and _execute handed Flow the operator's raw
deployment.json:

# origin/main:engine/runner_loop.py:876
config_path = self.config.data_dir / "deployment.json"
...
return bridge.run(bundle_dir, config_path, out_dir=run_dir, **kwargs)

Net: an administrator toggling a safety-critical setting in the dashboard changed
nothing about how a run executed. The control rendered, reported success, and
bound nothing — the exact failure the engine itself exists to prevent (reporting
success without confirming the effect), and the shape AGENTS.md §4 forbids: a
surface that looks like a safety boundary and is not one.

The existing 17 policy tests all passed, because every one of them tested
resolve_effective_policy rather than its effect.

What is now wired

RunnerService.bind_effective_policy() resolves the org's effective policy fresh
before every dispatched run — after digest/authorization revalidation,
before any GUI action — and hands Flow a policy-bound config staged privately
(0600, removed when the run ends) instead of the operator's file:

poll → lease → stage bundle → validate_dispatch → bind_effective_policy → execute
                                                   │
                                                   ├─ resolve (network → cache)
                                                   ├─ binding_safety(policy)      ── refuses
                                                   └─ apply_safety_policy(config) ── refuses

apply_safety_policy is strengthen-only — it can tighten a run, never relax
one. No existing guard was weakened to make the wiring simpler, and the policy
schema and dashboard are untouched.

Safety key Effect on the run
pixel_verify.consequential_policy requiredruntime.pixel_verify_enabled = true. disabled is the platform baseline, not a prohibition, so a locally-armed check stays armed.
model_calls.allowed_in_healthy_run falseruntime.allow_model_grounding = false (fully local). true is a permission, not an instruction to enable.
effect_verification.required_for_consequential, unverified_write.allow, identity_gate.strictness, halt_on_ambiguous Any at its strict value requires a production execution profile (effect contracts, identity coverage, settled frames, no blanket unverified-write approval). demo escalates to standard; an absent profile is left absent, because Flow resolves an omitted profile to regulated — stricter than anything we would write, so writing one in would weaken the run.
egress.artifact_policy Validated, deliberately not projected: artifact egress is governed by the upload path (engine/upload_manager.py, engine/hosted.py), not the replay runtime. Documented rather than silently claimed.

The grounding_model projection is deliberately not bound to runs: it is an
egress capability, and auto-enabling a capability from a cached policy is the
opposite of failing closed.

Fail-closed behaviour

The run refuses (ack outcome refused, evidence stream empty, flow engine
never invoked) when:

  1. source == "fail-closed-default" — no authoritative policy was ever
    obtained (no control plane, no cache). This is the important one. The safest
    values are populated, but they are the engine's guess at the org's posture,
    not the org's posture: an org that strengthened a key beyond baseline —
    e.g. pixel_verify.consequential_policy: required, whose safe default is
    disabled — would be silently run without it. That is precisely
    "degrading to allow" wearing a different hat. docs/POLICY_SYNC.md rule 5 and
    the cloud sync contract both already specified this refusal; nothing enforced
    it. A cached policy is authoritative and still runs offline.
  2. The safety block is absent, is not an object, or is missing a key.
  3. Any value falls outside SAFETY_VALUE_DOMAINS (mirroring the exact domains in
    openadapt-cloud src/lib/settingsRegistry.ts). harden_safety deliberately
    preserves whatever the server said — binding is where an unknown posture must
    stop a run.
  4. Policy resolution raises unexpectedly (defensive: the resolver is documented
    never to raise).
  5. There is no readable deployment config to bind the policy to, or it declares
    an execution profile that cannot be ranked (so the engine cannot prove it is
    not weakening it).

No path degrades to "run it anyway". Refusal reasons stay PHI-free — they name
keys, classes, and domains, never config contents, paths, or exception text
(asserted).

The test that would have caught this

TestSafetyPolicyBinding::test_admin_toggle_changes_what_the_run_executes
dispatches the same bundle twice with only the org's Tier-3 setting changed
between them, and asserts on the deployment config the flow bridge actually
received — not on what the resolver returned.

Demonstration that it fails without the fix. Reverting only the binding (the
policy is still resolved, exactly as on main, but never applied) and re-running:

$ pytest tests/test_engine/test_runner_loop.py -q -k SafetyPolicy
FAILED ...::TestSafetyPolicyBinding::test_admin_toggle_changes_what_the_run_executes
FAILED ...::TestSafetyPolicyBinding::test_model_call_prohibition_overrides_a_permissive_config
FAILED ...::TestSafetyPolicyBinding::test_demo_profile_is_escalated_never_lowered
FAILED ...::TestSafetyPolicyBinding::test_bound_config_does_not_outlive_the_run
FAILED ...::TestSafetyPolicyFailsClosed::test_refuses_when_no_authoritative_policy_exists
FAILED ...::TestSafetyPolicyFailsClosed::test_refuses_on_a_safety_value_outside_its_domain
FAILED ...::TestSafetyPolicyFailsClosed::test_refuses_on_a_missing_safety_key
FAILED ...::TestSafetyPolicyFailsClosed::test_refuses_when_resolution_raises
FAILED ...::TestSafetyPolicyFailsClosed::test_refuses_when_the_deployment_config_is_missing
FAILED ...::TestSafetyPolicyFailsClosed::test_refuses_on_an_unreadable_deployment_config
FAILED ...::TestSafetyPolicyFailsClosed::test_refuses_on_an_unrankable_execution_profile
11 failed, 5 passed, 22 deselected

with e.g.

>       assert flow.runtimes[1]["pixel_verify_enabled"] is True
E       KeyError: 'pixel_verify_enabled'

The change was then restored; the suite is green below.

Gates (exact pinned versions, uv sync --locked --extra dev)

$ uv run ruff check engine/ tests/ scripts/
All checks passed!
$ uv run pytest tests/ -q
610 passed, 2 skipped, 3 warnings

Baseline on origin/main was 573 passed / 2 skipped; +37 new tests, zero
regressions. (This repo's CI gates are ruff + pytest; there is no mypy gate.)

Fixture note

The runner rig now writes a deployment.json and injects a policy resolver.
Both reflect reality — a runner that can execute at all has a deployment config
and a reachable-or-cached policy — and the refusal tests remove or corrupt them
deliberately. No assertion was relaxed.

Scope / lane

openadapt-desktop, disjoint from PR #75 (mobile decision portal + pairing),
which owns engine/dispatch.py, engine/config.py, engine/portal/*,
src/*, and tests/test_portal/*. This PR touches only engine/policy.py,
engine/runner_loop.py, their two test modules, and docs/POLICY_SYNC.md.

Known remaining gap, deliberately out of scope: the local governed-run path
(engine/dispatch.py::_replay_or_run, the run=True branch at
dispatch.py:794) is still unbound — it builds its config through
prepare_flow_config(base_config, target) and never consults the policy. That
file is owned by PR #75 this cycle; binding it is a follow-up that should reuse
apply_safety_policy at the prepare_flow_config seam so both execution paths
share one projection.

🤖 Generated with Claude Code

https://claude.ai/code/session_01NyCHrzA1psrKMFfroYbzaM

The Cloud API, migration, dashboard, and `engine/policy.py`'s
`resolve_effective_policy` all shipped, but step 3 of 3 -- carrying the
resolved policy into execution -- never landed. On `origin/main`,
`engine/policy.py` is imported only by `engine/dispatch.py` (the UI verb
`get_effective_policy`); `engine/runner_loop.py` contains no reference to
policy at all, and `_execute` handed Flow the operator's raw
`deployment.json`. An admin toggling a safety-critical setting therefore
changed nothing about how a run executed: the control rendered, reported
success, and bound nothing.

Wire it:

* `engine/policy.py` gains `SAFETY_VALUE_DOMAINS` (the exact value domains
  from the cloud registry), `binding_safety()` -- the pre-run gate -- and
  `apply_safety_policy()`, a strengthen-only projection of the Tier-3 block
  onto a Flow deployment config: `pixel_verify.consequential_policy:
  required` arms `runtime.pixel_verify_enabled`; a prohibition on model
  calls forces `runtime.allow_model_grounding = false`; the effect
  verification / identity / halt-on-ambiguous / unverified-write keys
  require a production execution profile, escalating `demo` to `standard`
  while leaving an ABSENT profile absent (Flow defaults it to `regulated`,
  which is stricter than anything we would write).

* `engine/runner_loop.py` resolves the policy fresh before every dispatched
  run -- after digest/authorization revalidation, before any GUI action --
  and hands Flow a policy-bound config staged privately (0600, removed when
  the run ends) instead of the operator's file.

Fail closed. The run REFUSES (ack `refused`, flow never invoked) when the
policy is not authoritative (`source == "fail-closed-default"`: an org that
strengthened a key would otherwise be silently run without it), when a
safety key is missing or outside its domain, when resolution raises, or when
there is no bindable deployment config. No path degrades to "run it anyway".
Refusal reasons stay PHI-free.

Tests assert the EFFECT, not the resolver: the same bundle dispatched twice
across an admin toggle must produce a different deployment config at the
flow bridge, and every unenforceable-policy path must refuse before
execution.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NyCHrzA1psrKMFfroYbzaM
@abrichr
abrichr merged commit 0ceae7c into main Jul 27, 2026
15 checks passed
@abrichr
abrichr deleted the fix/bind-safety-policy-to-runs branch July 27, 2026 20:02
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.

2 participants