Fix #28: Relax hook policy for normal delivery workflows - #29
Conversation
|
Delivery owner report for #28. Run id: 5da148c5-6e29-4121-87dd-ce74fc6542c4. Hook doctor: router installed and current binding active; branch creation used recorded carryover fallback because old hook blocked git switch. Changes: quote-aware shell scanner, structured argv rawKind preserved, tiered allowlist for normal delivery, safe git gh npm pnpm agent-loop reviewer dispatch and localhost smoke commands, tightened destructive and data-exfiltration boundaries. Verification: pnpm exec vitest run plugins/autonomous-pr-loop/tests/hook-policy.test.ts passed 18 tests; pnpm lint passed; pnpm test passed 34 files and 395 tests; git diff --check passed; gitnexus detect_changes passed with expected HIGH hook-runtime impact. Internal tester PASS. Internal reviewer PASS after all P0 P1 P2 findings were fixed. Browser validation not required because this PR does not touch dashboard UI. |
|
Internal tester report. Run id: 5da148c5-6e29-4121-87dd-ce74fc6542c4. Reviewer: internal_tester. Role: tester. Model: GPT-5.3-Codex-Spark. Conclusion: PASS. Coverage: checked hook-policy diff and ran focused hook-policy tests, full pnpm test, and pnpm lint. Findings: P0 none, P1 none, P2 none, P3 one tsx path traversal risk. Resolution: fixed in this PR by normalizing repo-owned tsx entrypoints and rejecting parent traversal, with regression coverage. |
|
Internal reviewer report. Run id: 5da148c5-6e29-4121-87dd-ce74fc6542c4. Reviewer: internal_reviewer. Role: code and security review. Model: GPT-5.5. Conclusion: PASS after fixes. Initial findings included shell command substitution bypass, spoofable reviewer dispatch path, gh write and GraphQL over-broad access, local path exfiltration, npm pnpm git and curl over-broad allowlists. Resolution: all P0 P1 P2 findings fixed in this PR with focused regression tests. Final review result: P0 none, P1 none, P2 none, P3 none. |
|
Update after Claude ACP fixes. Run id: 5da148c5-6e29-4121-87dd-ce74fc6542c4. Added stricter scope guards for curl allowlist, gh repo and hostname flags, git global options, env scope variables, npm and pnpm registry flags, package install specs, and reviewer dispatch path portability. Re-ran pnpm exec vitest run plugins/autonomous-pr-loop/tests/hook-policy.test.ts, pnpm lint, pnpm test, git diff --check, and gitnexus detect_changes. All passed. Browser validation remains not required because no UI changed. |
|
Claude ACP review report. Run id: 5da148c5-6e29-4121-87dd-ce74fc6542c4. Reviewer: claude_acp. Sessions: e95413f9-0dac-4b3c-83b0-77c455eb9196, 1cca9cbc-15de-4153-9901-e0eced16f86b, bcded41e-8ccb-4d31-98c2-81823d3781a0. Initial conclusion: BLOCK with P1 findings for curl SSRF and file IO bypasses, gh repo/hostname scope, git global scope, env scope, npm registry scope. Fixes pushed in follow-up commits f4bdc0c and 33e4a89. Final narrow re-review conclusion: PASS. Remaining findings: P0 none, P1 none, P2 none, P3 none. Verification cited by reviewer: focused hook-policy test passed 18 of 18 and dist/pre-tool-use.js is synchronized with source. |
|
CI follow-up. Run id: 5da148c5-6e29-4121-87dd-ce74fc6542c4. GitHub Actions failed because the hook-policy test used a local mac-mini HOME path for trusted skill wrappers. Fixed the test to derive trusted skill paths from process.env.HOME, preserving the same policy behavior while making CI portable. Local verification after this fix: focused hook-policy tests passed 18 of 18, pnpm lint passed, pnpm test passed 34 files and 395 tests, git diff --check passed, gitnexus detect_changes passed with LOW test-only impact. |
|
Final hook-policy follow-up. Run id: 5da148c5-6e29-4121-87dd-ce74fc6542c4. While attempting the merge, local policy correctly required an explicit scoped gh merge command, but the merge-specific allowlist did not skip the already-validated repo flag. Fixed matchesGhPrMergeAllowlist to allow --repo, -R, --repo=, -R=, and attached -R forms only after matchesGhRepoScope has validated the current repo. Local verification passed: focused hook-policy tests, pnpm lint, pnpm test, git diff --check, and gitnexus detect_changes. |
Fixes #28. Run id: 5da148c5-6e29-4121-87dd-ce74fc6542c4. Summary: relax hook policy for normal delivery workflows while tightening shell expansion, repo scope, reviewer dispatch, curl, npm, pnpm, git, and gh safety boundaries. Verification: pnpm exec vitest run plugins/autonomous-pr-loop/tests/hook-policy.test.ts passed; pnpm lint passed; pnpm test passed 34 files and 395 tests; gitnexus detect_changes passed with expected HIGH hook-runtime impact. Internal tester PASS. Internal reviewer PASS after P0/P1/P2 fixes. Browser validation not required because no dashboard UI changes.