policy: repair required deny tool findings#99700
Conversation
|
Codex review: needs maintainer review before merge. Reviewed July 6, 2026, 12:11 PM ET / 16:11 UTC. Summary PR surface: Source +117, Tests +141, Docs +7. Total +265 across 6 files. Reproducibility: not applicable. as a bug reproduction; this PR adds a new doctor repair capability. The PR body provides after-fix source-entry proof for the repair path with isolated config and post-write re-detection. Review metrics: 1 noteworthy metric.
Stored data model Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Maintainer decision needed
Security Review detailsBest possible solution: Land the deny-list repair only after maintainer acceptance of the workspaceRepairs-gated config/security behavior, keeping the exact-path mutation and inherited-root scoped skip intact. Do we have a high-confidence way to reproduce the issue? Not applicable as a bug reproduction; this PR adds a new doctor repair capability. The PR body provides after-fix source-entry proof for the repair path with isolated config and post-write re-detection. Is this the best way to solve the issue? Yes, subject to maintainer acceptance: using the existing Policy AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 32cc9b08e6b3. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +117, Tests +141, Docs +7. Total +265 across 6 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
Review history (20 earlier review cycles; latest 8 shown)
|
|
@clawsweeper re-review I restacked the policy series onto current Fresh proof from the top of the stack (
|
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
Codex review of the Required Denies PR. Findings:
That means a scoped policy such as requiring one extra denied tool for one agent can be repaired by appending the tool to root Best-fix verdict: not the best fix yet. For scoped agent findings, the repair target should be the scoped agent config path, or the inherited/global-source case should stay skipped/review-required instead of mutating shared root config. Evidence checked: |
|
Addressed Omar's scoped required-deny finding in |
|
@clawsweeper re-review Reviewer-requested fixes are pushed on this updated head. Fresh local proof before push: focused policy/core/CLI/tooling Vitest passed, |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
Updated this PR body with current stack heads and final-stack proof after the #99686 proof refresh. The foundation proof now shows policy-owned policy.fixRecommendation in public policy JSON and recommendation-free attestation hashing, and this PR includes that fix via the forward-merged stack. @clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
8a22456 to
c7ab05e
Compare
c7ab05e to
4f17843
Compare
4f17843 to
18ff62b
Compare
|
Merged via squash.
|
Summary
tools.denyor agenttools.denyconfig path.workspaceRepairs=trueopt-in inherited from policy: repair automatic narrowing findings #99690.tools.deny.Stack
policy: repair automatic narrowing findings).cd11ce139ec649e41acb8ce76020a204e474c943.Validation
node scripts/check-docs-mdx.mjs docs README.mdpassed: 691 files.git diff --checkpassed.$env:OPENCLAW_VITEST_MAX_WORKERS='1'; node scripts/run-vitest.mjs extensions/policy/src/doctor/register.test.ts extensions/policy/src/doctor/metadata.test.ts src/flows/doctor-repair-flow.test.ts --reporter=dotpassed: 2 Vitest shards / 304 tests.node scripts/run-tsgo.mjs -p tsconfig.extensions.json --incremental --tsBuildInfoFile .artifacts/tsgo-cache/extensions.tsbuildinfopassed.$env:OPENCLAW_OXLINT_SKIP_PREPARE='1'; node scripts/run-oxlint.mjs --tsconfig config/tsconfig/oxlint.extensions.json extensions/policy/src/doctor/fix-metadata.ts extensions/policy/src/doctor/metadata.test.ts extensions/policy/src/doctor/automatic-repairs.ts extensions/policy/src/doctor/register.test.tspassed.node --import tsx scripts/check-policy-config-coverage.ts --check --jsonpassed with no unclassified, unmatched, or stale monitored paths.Real behavior proof
Behavior or issue addressed:
doctor --fixrepairs required deny-tool Policy findings by adding missing required tools to the reported deny-list config path, while preserving existing entries and skipping scoped inherited-root writes.Real environment tested:
Windows PowerShell in
C:\src\openclaw-forkon PR headcd11ce139ec649e41acb8ce76020a204e474c943. The proof used an isolated tempOPENCLAW_CONFIG_PATH/OPENCLAW_STATE_DIR, registered the real Policy doctor checks from source, ran the realrunDoctorHealthRepairsrepair flow, persisted throughreplaceConfigFile, and rechecked the same Policy finding.Exact steps or command run after this patch:
openclaw.jsoncwithplugins.entries.policy.config.workspaceRepairs=trueandtools.deny=["read"].policy.jsoncwithtools.denyTools=["exec"].node --import tsx --input-type=modulesource proof that invokesregisterPolicyDoctorChecks, selectspolicy/tools-required-deny-missing, runsrunDoctorHealthRepairs, persists withreplaceConfigFile, then re-detects the finding.Evidence after fix:
Observed result after fix:
The seeded required-deny finding was repaired by adding
execto roottools.deny, the config was written to disk through OpenClaw's config mutation path, repair validation reported zero remaining findings, and a post-write re-detect ofpolicy/tools-required-deny-missingreturnedAFTER_FINDINGS=0.What was not tested:
A full packaged/dev CLI
openclaw doctor --fixsmoke was attempted with isolatedOPENCLAW_CONFIG_PATHandOPENCLAW_STATE_DIR, but local Windows dev CLI startup did not return before a 240s timeout. No live provider run or live LLM call was run. Targetedpnpm exec oxfmt --check ...was also attempted locally but timed out before producing a result;git diff --checkand targeted oxlint passed.