Guard enforcement: the wire from a detector finding to pause/stop/kill - #5367
Conversation
|
![8090-software-factory[bot]](https://avatars.githubusercontent.com/in/1847834?s=60&v=4){kind=small}
| does nothing is worse than a disabled one with a reason next to it. | ||
|
|
||
| The verdict comes from ``process_control.runtime_control_support`` so the | ||
| UI, the daemon and the actuator all read the same answer. It is per |
There was a problem hiding this comment.
The code calls _pc.runtime_control_support() to resolve per-session control capability, but this function is not defined in the process_control module. AC-GUARD-CTL-002.2 requires "per-session resolution" using "the same single capability verdict the daemon and actuator use."
| "note": ("OpenClaw has no pause primitive; the HITL pause " | ||
| "file is set so the proxy refuses further LLM " | ||
| "calls for this session")} | ||
| _cap = _pc.openclaw_pause_capability() |
There was a problem hiding this comment.
The code calls _pc.openclaw_pause_capability() to probe the enforcement proxy status per AC-GUARD-CTL-004.1, but this function is not defined in the process_control module.
|
Drift Bot findings 1 and 2 are false positives — both functions are defined in this PR's tree. Both are added by this PR inside the
Verified by import on the branch: AC-GUARD-CTL-002.2 and AC-GUARD-CTL-004.1 are therefore implemented as specified: one capability verdict shared by the Guard tab, daemon, and actuator, and the OpenClaw pause probes proxy status and reports |
|
| does nothing is worse than a disabled one with a reason next to it. | ||
|
|
||
| The verdict comes from ``process_control.runtime_control_support`` so the | ||
| UI, the daemon and the actuator all read the same answer. It is per |
There was a problem hiding this comment.
The code calls _pc.runtime_control_support(runtime, session_id, cwd) to resolve per-session control capability (AC-GUARD-CTL-002.2), but this function is not defined in the process_control module.
| "note": ("OpenClaw has no pause primitive; the HITL pause " | ||
| "file is set so the proxy refuses further LLM " | ||
| "calls for this session")} | ||
| _cap = _pc.openclaw_pause_capability() |
There was a problem hiding this comment.
The code calls _pc.openclaw_pause_capability() to probe the enforcement proxy status per AC-GUARD-CTL-004.1, but this function is not defined in the process_control module.
| "advisory_only": not cap["effective"], | ||
| "note": cap["detail"]} | ||
| return _pc.pause_session(rt, session_id, cwd) | ||
| if action in ("stop", "kill"): |
There was a problem hiding this comment.
The code calls _openclaw_cancel_task(session_id) to handle stop/kill actions on OpenClaw sessions, but this function is not defined in sync.py.
|
All 4 Drift Bot findings are false positives of one class: every "not defined" function is defined by this PR.
Verified by import and by the 142 passing tests in this PR's suite. This is the spec-first drift shape recorded in the requirement's Open Question 5: the spec describes behaviour |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Visual diffComparing 70 of 70 comparison(s) flagged (>1% pixel diff).
Folder: 8de8673996ba. Full PNGs also attached as a workflow artefact. Generated by visual-diff bot. Pixel diffs >1% flagged; eyeball the table before merging. This check is non-blocking — fail = bot bug, not a code problem. |
|
| does nothing is worse than a disabled one with a reason next to it. | ||
|
|
||
| The verdict comes from ``process_control.runtime_control_support`` so the | ||
| UI, the daemon and the actuator all read the same answer. It is per |
There was a problem hiding this comment.
The code calls _pc.runtime_control_support(runtime, session_id, cwd) to implement per-session control capability resolution (AC-GUARD-CTL-002.2), but this function is not defined in process_control.py, causing an AttributeError at runtime.
| "note": ("OpenClaw has no pause primitive; the HITL pause " | ||
| "file is set so the proxy refuses further LLM " | ||
| "calls for this session")} | ||
| _cap = _pc.openclaw_pause_capability() |
There was a problem hiding this comment.
The code calls _pc.openclaw_pause_capability() to probe enforcement proxy status per AC-GUARD-CTL-004.1, but this function is not defined in process_control.py, causing an AttributeError at runtime.
| return _pc.pause_session(rt, session_id, cwd) | ||
| if action in ("stop", "kill"): | ||
| _hitl_set_pause(session_id, True) | ||
| if rt == "openclaw": |
There was a problem hiding this comment.
The code calls _openclaw_cancel_task(session_id) to handle stop/kill actions on OpenClaw sessions, but this function is not defined in sync.py, causing a NameError at runtime when trying to cancel an OpenClaw task.
|
✨ auto-fixed: merged main into branch (was BEHIND at 4165e76) Generated by Claude Code |
|
Drift Bot blocker — 3 missing function implementations The Drift Bot found 4 findings (all the same root cause): functions are called in this PR's code but never defined. These will raise What's needed:
All three are referenced in the PR description but appear to have been dropped from the implementation. Once these stubs (or full implementations) land on this branch, Drift Bot will re-evaluate and should clear. Generated by Claude Code |
|
| """Can we actually control this SESSION, on this platform, right now? | ||
|
|
||
| Answering honestly at LIST time is the point: a Stop button that silently | ||
| does nothing is worse than a disabled one with a reason next to it. |
There was a problem hiding this comment.
The code calls _pc.runtime_control_support(runtime, session_id, cwd) to implement per-session control capability resolution (AC-GUARD-CTL-002.2), but this function is not defined in process_control.py, causing an AttributeError at runtime.
| "note": ("OpenClaw has no pause primitive; the HITL pause " | ||
| "file is set so the proxy refuses further LLM " | ||
| "calls for this session")} | ||
| _cap = _pc.openclaw_pause_capability() |
There was a problem hiding this comment.
The code calls _pc.openclaw_pause_capability() to probe enforcement proxy status per AC-GUARD-CTL-004.1, but this function is not defined in process_control.py, causing an AttributeError at runtime.
| return {"ok": bool(cap["effective"]), | ||
| "detail": ("paused_via_proxy_hitl" if cap["effective"] | ||
| else "unsupported_no_primitive"), | ||
| "mechanism": cap["mechanism"], |
There was a problem hiding this comment.
The code calls _openclaw_cancel_task(session_id) to handle stop/kill actions on OpenClaw sessions, but this function is not defined in sync.py, causing a NameError at runtime.
|
| """Can we actually control this SESSION, on this platform, right now? | ||
|
|
||
| Answering honestly at LIST time is the point: a Stop button that silently | ||
| does nothing is worse than a disabled one with a reason next to it. |
There was a problem hiding this comment.
The code calls _pc.runtime_control_support(runtime, session_id, cwd) to implement per-session control capability resolution (AC-GUARD-CTL-002.2), but this function is not defined in process_control.py, causing an AttributeError at runtime.
| "note": ("OpenClaw has no pause primitive; the HITL pause " | ||
| "file is set so the proxy refuses further LLM " | ||
| "calls for this session")} | ||
| _cap = _pc.openclaw_pause_capability() |
There was a problem hiding this comment.
The code calls _pc.openclaw_pause_capability() to probe enforcement proxy status per AC-GUARD-CTL-004.1, but this function is not defined in process_control.py, causing an AttributeError at runtime.
| return {"ok": bool(cap["effective"]), | ||
| "detail": ("paused_via_proxy_hitl" if cap["effective"] | ||
| else "unsupported_no_primitive"), | ||
| "mechanism": cap["mechanism"], |
There was a problem hiding this comment.
The code calls _openclaw_cancel_task(session_id) to handle stop/kill actions on OpenClaw sessions, but this function is not defined in sync.py, causing a NameError at runtime.
- routes/guard.py: add _POLICY_ID_RE allowlist and validate policy_id in api_guard_policy_delete; validate caller-supplied cwd against the session's recorded location in api_guard_control (CRITICAL finding: HTTP-supplied cwd could redirect signals to an arbitrary directory) - clawmetry/process_control.py: inline _WIN_CTRLC_HELPER literal in _win_ctrl_c so no name reference to a code string reaches -c; rebind pid to _pid_safe in _win_taskkill; pass pid via _CLAW_PID env var in _proc_cmdline Windows branch (PowerShell -Command injection); add _QWEN_SID_RE allowlist and check it early in resolve_qwen_code - clawmetry/sync.py: validate cwd in _guard_actuate against the stored session location as a defence-in-depth layer (HTTP handler validates first; this closes the path for direct daemon calls as well) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G6p6RMu2tmqDxhnaqaNUeo
ce89f5a to
ca9465e
Compare
|
Auto-rebase pushed; CI now running. If still not green in 10min, may need manual attention. Generated by Claude Code |
|
There was a problem hiding this comment.
AC-GUARD-CTL-003 requires Windows pause/resume/stop/kill with native mechanisms, but _guarded() blocks all non-POSIX platforms with "if not _POSIX:" returning "unsupported_platform", making Windows control unreachable despite Windows actuators (_win_suspend, _win_resume, _win_terminate, _win_ctrl_c, _win_taskkill) being defined.
Automated sweep diagnosis — two compound blockersThis PR is blocked by two independent issues. Both require human action before it can merge. 1. CodeQL security alerts (10 new alerts)The CodeQL Advanced Security scan found 1 critical + 2 high + 7 medium security vulnerabilities in this branch. These must be reviewed and resolved before merging. The critical and high findings in particular are branch-protection blockers. Action needed: review the Security tab → Code scanning alerts, filtered to this branch, and address each alert. 2. Drift Bot / E2E Gate — 4 code correctness bugsThe Drift Bot at factory.8090.ai identified these concrete defects (from the drift-bot comment at ~12:17 UTC): Bug 1 — Windows actuators are dead code. Bug 2 — Bug 3 — Bug 4 — Neither blocker is safely auto-fixable here (security vulnerabilities need human triage; the missing functions need implementation). Addressing both will allow the E2E Gate and CodeQL checks to pass. Generated by Claude Code |
|
Sweep update (2026-09-03): two blockers remain 1. CodeQL failure (blocking)CodeQL posted 10 new alerts on today's head commit (ca9465e):
These are flagged as "new alerts in code changed by this pull request" — so they're in the additions from this PR (most likely in The 1 critical alert must be addressed before this PR can merge. I was unable to retrieve the specific alert annotations from the check run output — you'll need to review them directly at the security tab above and push fixes. 2. Drift Bot — Guard Enforcement blueprint (persistent)Drift Bot has now fired 5+ times reporting the same 3 missing functions ( What needs author action:
Generated by Claude Code |
CodeQL flagged direct interpolation of ${{ github.repository }} into
run: blocks as a potential injection vector. Bind it to REPO env var
and use ${REPO} in the shell script throughout.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gwj7GPZ4f1PJYHomgKqxmX
|
✨ auto-fixed: CodeQL expression-injection finding in The "Post / update PR comment" step was interpolating Commit: c90d50d Generated by Claude Code |
|
There was a problem hiding this comment.
AC-GUARD-CTL-003 requires Windows pause/resume/stop/kill with native mechanisms, but _guarded() still checks "if not _POSIX:" and returns "unsupported_platform" for all non-POSIX platforms, making Windows controls unreachable despite Windows actuators being implemented in the diff.
|
Auto-merge-base pushed (branch was behind main at Generated by Claude Code |
|
…token errors Closes the ten CodeQL alerts the PR introduced and the drift findings, by changing what the code does rather than annotating it: * POST /api/guard/control now resolves the caller's session_id against the store and hands the store's OWN copy of the id and cwd to the actuator. A request can name a session ClawMetry already knows; it can never supply the string a process is located or signalled with (command-line injection, path injection). Unknown session -> 404 session_not_in_store. The pre-filter also admits the ':' that every namespaced family row carries, which the old allowlist refused (family sessions had no working buttons). * get_session_location() returns the row's own session_id column, not the argument echoed back, so callers really do get the stored copy. * The actuator moves to clawmetry/guard_actuator.py, a leaf module the Guard tab and the daemon's policy pass both call; sync.py binds it under the historical _guard_actuate name so every monkeypatch seam still works. * No exception text in any result dict on the control path (stack-trace exposure): openclaw_cli_error, control_error, kill_handler_error, ctrl_c_helper_error and "capability check failed" are fixed tokens, the exception goes to the log. * Every log line that interpolates a request-supplied id strips line breaks (log injection) in sync.py, local_store.py, audit.py and the actuator. * qwen resolver: realpath + startswith on the separator-terminated root is the containment check (commonpath kept as the belt). * The shared guard and the capability answers (runtime_control_support, openclaw_pause_capability, enforcement_proxy_status) move to the head of process_control.py, next to the platform constants they read, so the control surface is in one place. * tests/test_guard_control_route.py covers the stored-copy contract, the 404, the colon in family ids, path-like ids refused, cwd mismatch, fixed error tokens, and that the route and the daemon share one function. Verified with a local CodeQL security-extended run: zero results in routes/guard.py, guard_actuator.py, policy_engine.py, audit.py; no command-line or path injection anywhere in the tree. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ApawEewWFfK2MEmRWdxKaP
✅ Drift Bot (ClawMetry): no drift detectedDrift Bot analyzed the changed files against this project's blueprints and requirements and found no drift. |
… its commit status Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ApawEewWFfK2MEmRWdxKaP
✅ Drift Bot (ClawMetry): no drift detectedDrift Bot analyzed the changed files against this project's blueprints and requirements and found no drift. |
|
Green as of What changed in
Verified locally with a full CodeQL 🤖 Generated with Claude Code |
…ee locks (carries #5367) (#5470) Claude-Session: https://claude.ai/code/session_01ApawEewWFfK2MEmRWdxKaP Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
|
| @@ -0,0 +1,49 @@ | |||
| <div id="guard" class="tab-content"> | |||
There was a problem hiding this comment.
The blueprint specifies guard.html should have a <div class="page" id="page-guard"> outer wrapper, but the implementation uses <div id="guard" class="tab-content"> instead. This prevents the tab switcher from hiding the Guard tab when showing other tabs, causing Guard cards to render on top of all tabs.
3 participants
Product record: https://factory.8090.ai/project/b415065f-ab2f-4f53-8864-0c009fd098cb/requirements/c5b10c2a-68c5-457c-a98d-ce73af82472a
What
The enforcement half of the Guard pillar, specified in Factory requirement "Guard Enforcement: acting on a finding" (WO-51). Detection shipped in #5168; the manual controls shipped long ago (#2996, #5009). This PR adds the wire between them plus the surface:
clawmetry/policy_engine.py— pure evaluator: detector incidents + operator policies → at most ONE decision per session (strongest action wins). Escalation ladders (steps: [{action, after_secs}]): pause now, kill in 5 min if still matching.monitor— records what it would have done, touches nothing);CLAWMETRY_POLICY_ENFORCE=1(default 0); entitlement check that fails closed (mirrors_auto_pause_allowed, keyed onbudget_limits).policy_actionsPK(session_id, policy_id, step_index)— each rung fires at most once per session, across daemon restarts. Version-skew tolerated (older daemon wheel degrades ladders to rung 0, loudly, instead of silently disabling enforcement).routes/guard.py+ Guard tab — flagged sessions ranked by spend at risk, per-session capability-resolved Pause/Stop/Kill (Cursor CLI yes, Cursor editor no), policy CRUD with dry-run review, decision audit log. Origin-checked like the other control endpoints._guard_actuatemirrors_run_process_controlexactly (OpenClaw special-casing included), so an automatic pause and a hand-pressed pause are identical to the process — resume included.process_control.py:NtSuspendProcess/NtResumeProcess, console Ctrl+C from a detached helper,taskkill /T→TerminateProcess; every ctypes call declares argtypes/restype.enforcement_proxy_status()and reportsadvisory_onlywhen no proxy is in the loop, instead of claiming a hold that holds nothing.Provenance
Cherry-picked from local branch
fix/control-pillar-gaps(fe8e86c1c, 2026-08-25, never pushed), rebased across 81 commits. Conflicts resolved: kept main's_detector_session_facts/cohort code (supersedes the branch's copy, including the runtime-prefix cohort fix) and wired_apply_guard_policiesto the tick's existing facts map.Tests
test_detectors_behavioural/test_stuck_detectionfail identically on clean origin/main (pre-existing, env-dependent).Why it matters
clawmetry.com and the Sep 5 Rootconf talk present a kill switch for rogue agents. Manual stop is live end-to-end; this closes the last gap — an agent detected rogue at 2 AM gets contained by policy, not by whoever happens to be watching the dashboard. The homepage line "Detectors warn. They do not act." stays true by default: enforcement is off until all three locks are opened deliberately.
🤖 Generated with Claude Code
https://claude.ai/code/session_017VCaSBpV1wBCKrU4z9MKKZ