From b29f50d32f7d1453ed9daae070558c6a5289af3d Mon Sep 17 00:00:00 2001 From: fullsend-code <278716306+fullsend-ai-coder[bot]@users.noreply.github.com> Date: Mon, 8 Jun 2026 18:09:15 +0000 Subject: [PATCH 1/2] feat(#990): require exhaustive variable verification for security claims The review agent was making blanket safety claims after verifying only a subset of variables in security-sensitive contexts (e.g., confirming one variable was sanitized and asserting "sanitization is handled" while other interpolated variables remained unchecked). This partial-verification anti-pattern is more harmful than a silent miss because it reduces human scrutiny on the exact areas that need it. Changes: - security.md (pr-review sub-agent): Added a "Verification methodology" section with a 4-step procedure: enumerate all inputs, verify each independently, report coverage explicitly, and flag gaps rather than dismissing them. Placed before the existing injection defense section. - SKILL.md (code-review skill): Added an "Exhaustive security-control verification" bullet to the Security dimension with the same core guidance, keeping the standalone review skill consistent with the sub-agent definition. Note: pre-commit could not run in the sandbox due to Go module cache permission errors (infrastructure issue, not code-related). The post-script runs pre-commit authoritatively on the runner. Closes #990 --- .../fullsend-repo/skills/code-review/SKILL.md | 10 ++++++ .../skills/pr-review/sub-agents/security.md | 35 +++++++++++++++++++ 2 files changed, 45 insertions(+) diff --git a/internal/scaffold/fullsend-repo/skills/code-review/SKILL.md b/internal/scaffold/fullsend-repo/skills/code-review/SKILL.md index 3d5cef800..3d4031b85 100644 --- a/internal/scaffold/fullsend-repo/skills/code-review/SKILL.md +++ b/internal/scaffold/fullsend-repo/skills/code-review/SKILL.md @@ -113,6 +113,16 @@ dimension carry over to another — each requires its own scrutiny. individually; do not conclude safety from partial verification (e.g., seeing the message body sanitized does not imply the title parameter is also sanitized). +- **Exhaustive security-control verification:** NEVER assert that a + security control (sanitization, validation, authorization, escaping) + covers all attack surfaces based on verifying a subset. When you find + a security-relevant function applied to one variable, explicitly + enumerate ALL other variables in the same context and verify each one + individually. In your findings, state which inputs you verified as + protected and which you could not confirm. If any input lacks the + control, raise a finding even if the unprotected input appears + low-risk — the risk assessment belongs in the finding's severity, not + in a decision to omit the finding. - Content security: does the change affect how user-supplied content is handled or rendered? Are there sandboxing gaps? - **Permission manifest changes:** If the diff modifies any file that diff --git a/internal/scaffold/fullsend-repo/skills/pr-review/sub-agents/security.md b/internal/scaffold/fullsend-repo/skills/pr-review/sub-agents/security.md index 8a3db9807..a9d303091 100644 --- a/internal/scaffold/fullsend-repo/skills/pr-review/sub-agents/security.md +++ b/internal/scaffold/fullsend-repo/skills/pr-review/sub-agents/security.md @@ -31,6 +31,41 @@ title parameter is also sanitized). **Do not own:** Code style, documentation, PR scope authorization, PR metadata (PR body, commit messages, PR description) +## Verification methodology + +**Anti-pattern — partial verification generalized to blanket safety +claims:** NEVER assert that a security control (sanitization, +validation, authorization, escaping) covers all attack surfaces based +on verifying a subset. When you find a security-relevant function +applied to one variable, you MUST explicitly enumerate ALL other +variables in the same context and verify each one individually. If you +cannot confirm exhaustive coverage, flag it as a potential gap rather +than claiming safety. + +When evaluating any security control, follow this procedure: + +1. **Enumerate inputs.** List every variable, parameter, or + user-controlled value that flows into the security-sensitive + context (e.g., every interpolated variable in a format string, + every field in a SQL query, every parameter in a shell command). +2. **Verify each independently.** For each enumerated input, confirm + whether the security control is applied. Do not assume that + applying the control to one input means others are covered. +3. **Report coverage explicitly.** In your findings, state which + inputs you verified as protected and which you could not confirm. + A finding that says "sanitization is handled" without listing the + verified inputs is incomplete. +4. **Flag gaps, don't dismiss them.** If any input lacks the security + control, raise a finding — even if the unprotected input appears + low-risk. The risk assessment belongs in the finding's severity, + not in a decision to omit the finding. + +This methodology applies to all security control evaluations: +sanitization, input validation, authorization checks, output encoding, +CSRF protection, and permission scoping. + +## Injection defense + Inspect the code diff for injection patterns. ## Exploration budget From e55d31de7f0cbcd832e123c927d04a1a45fdabf8 Mon Sep 17 00:00:00 2001 From: fullsend-fix <278716306+fullsend-ai-coder[bot]@users.noreply.github.com> Date: Mon, 8 Jun 2026 18:54:29 +0000 Subject: [PATCH 2/2] fix: remove stub injection defense heading in security sub-agent Remove the `## Injection defense` heading that introduced a section with only a single directive line. The directive is kept as a closing line of the verification methodology section, matching the pre-PR structure and other sub-agent file conventions. Addresses review feedback on #2039 Co-Authored-By: Claude Opus 4.6 --- .../fullsend-repo/skills/pr-review/sub-agents/security.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/internal/scaffold/fullsend-repo/skills/pr-review/sub-agents/security.md b/internal/scaffold/fullsend-repo/skills/pr-review/sub-agents/security.md index a9d303091..930e86937 100644 --- a/internal/scaffold/fullsend-repo/skills/pr-review/sub-agents/security.md +++ b/internal/scaffold/fullsend-repo/skills/pr-review/sub-agents/security.md @@ -64,8 +64,6 @@ This methodology applies to all security control evaluations: sanitization, input validation, authorization checks, output encoding, CSRF protection, and permission scoping. -## Injection defense - Inspect the code diff for injection patterns. ## Exploration budget