fix(rules): SD022 precision — vendored subtrees, archives, comments - #660
Conversation
Sibling imprecisions to the expect_in_hot_path fix. Both found while triaging
gitbot-fleet 2026-07-29, where they accounted for the only 'critical' security
finding and the only 'high' -- and both were wrong.
1. actions_expression_injection (was :critical)
The detector was run:[\s\S]*?\$\{\{ github... \}\} and [\s\S]*? crosses
step boundaries. In repo-integrity-guard.yml it spanned from a run: in ONE
step to a PR_TITLE: binding in the NEXT step's env: block, and reported
script injection against a file that does precisely what the rule's own
remediation text asks: bind untrusted context via env:, consume it as a
quoted shell variable through grep -F, never interpolate it.
Now the match may not cross a new step head or a sibling mapping key at
step indent, so it stays inside one run block.
Verified: repo-integrity-guard.yml 1 -> 0 findings; a constructed workflow
that really does interpolate github.event.pull_request.title inside run:
still yields 1.
2. secret_action_without_presence_gate (was :high)
Accepted two gate idioms but not a third that is arguably the cleanest: a
job-level env: binding a presence boolean, with steps gating on it.
env:
HAS_TOKEN: ${{ secrets.FARM_DISPATCH_TOKEN != '' }}
steps:
- if: env.HAS_TOKEN == 'true'
instant-sync.yml does exactly this on every step and adds an else branch,
and was still reported.
Verified: instant-sync.yml 1 -> 0 findings; a step consuming the same secret
with no gate at all still yields 1.
Both keep their teeth and lose their false alarms.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ents
Third and final imprecision from the gitbot-fleet triage. All 11 SD022 findings
there were false positives, in three classes:
1. NESTED PROJECT ROOTS (10 of 11). bots/echidnabot/ is a vendored copy of the
standalone echidnabot repo and carries its own Cargo.toml. Its docs describe
THAT project -- CANONICAL_SOURCE.md documents src/abi/ and says in the same
table that the ABI namespace is 'owner-managed in standalone; fleet does not
need it for deploy'. src/abi/ exists upstream and is deliberately absent from
the mirror, so the doc is correct and the finding was not actionable here:
the mirror cannot fix a layout it does not own. Docs inside a subtree with
its own build manifest are now out of scope; a root manifest still counts,
so a real single-project repo is unaffected.
2. ARCHIVES. docs/archive/ records the tree AS IT WAS. A path valid when
written is not drift, and 'fixing' it would falsify the record.
3. COMMENTS. INTENT.contractile fired on
; "src/abi/ - formal proofs, changes require re-verification"
a commented-out example inside a '; *REMINDER: ...*' template block. Same
family as the known unwrap-rule-matches-comments defect. Full-line comments
are now stripped first -- conservatively: ';' for scheme-shaped
contractile/twasm, '#' and ';' for TOML-shaped toml/a2ml, and Markdown /
AsciiDoc left alone since '#' there is a heading that may carry a path.
Verified both directions. Real repo: gitbot-fleet 11 -> 0. Purpose-built control
repo, where the rule MUST still fire:
README.md -> src/parser/ FIRES (directory genuinely absent)
NOTES.contractile -> src/alsomissing/ FIRES (live line, not a comment)
NOTES.contractile -> src/commented-away/ silent (commented out)
src/real/ silent (exists)
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
|
Note Automatic reviews are paused because your trial's included automatic processing has been used for this period. Upgrade now, or comment "Gitar review" to run a review anytime. Code Review 👍 Approved with suggestions 0 resolved / 2 findingsRefines SD022 precision to ignore vendored subtrees, archives, and commented-out templates. Consider evaluating the nested_project_doc? suppression and env_boolean_gate? file-wide matching findings.
💡 Edge Case: nested_project_doc? suppresses drift in first-party umbrella/monorepo subprojects📄 lib/rules/structural_drift.ex:1095-1106
💡 Edge Case: env_boolean_gate? matches file-wide, can mask a genuinely ungated step📄 lib/rules/workflow_audit.ex:1012-1025 📄 lib/rules/workflow_audit.ex:998-1000
🤖 Prompt for agentsOptionsDisplay: compact → Showing less information. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Gitar |
…em repos hypatia's dogfood-gate.yml pinned hyperpolymath/a2ml-validate-action and k9-validate-action. Both repositories are DELETED (verified 404). An unresolvable uses: ref produces NO check run at all, so 'Validate A2ML manifests' and 'Validate K9 contracts' could never report -- which is what was blocking this PR. The actions moved rather than died: hyperpolymath/a2ml-ecosystem/validate-action hyperpolymath/k9-ecosystem/validate-action SHAs are those already proven green in the-nash-equilibrium#83; both verified to resolve and to contain validate-action/action.yml at that exact ref. Same fix as gitbot-fleet#451. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 participant
Third imprecision from the gitbot-fleet triage. All 11 SD022 findings there were false positives, in three classes:
bots/echidnabot/is a vendored copy of the standalone repo, with its ownCargo.toml. ItsCANONICAL_SOURCE.mddocumentssrc/abi/and states in the same table that the ABI namespace is "owner-managed in standalone; fleet does not need it for deploy". The doc is correct; the mirror cannot fix a layout it does not own.docs/archive/records the tree as it was — "fixing" it would falsify the record.INTENT.contractilefired on a commented-out template example.Verified both directions. gitbot-fleet 11 → 0, and on a purpose-built control repo:
README.md→src/parser/(absent).contractilelive line →src/alsomissing/.contractilecommented linesrc/real/(exists)🤖 Generated with Claude Code