docs(sast): fleet SAST gate canary results#42
Open
escote-tatari wants to merge 1 commit into
Open
Conversation
Evidence companion to 2026-07-03-fleet-sast.md. Records the two-test-bed canary that resolved the gate crux: the required-workflow gate blocks on the workflow RUN conclusion, not per-job check-runs, so continue-on-error protects merges for findings AND guarded crashes. Documents the residual blocking modes (unparseable YAML, unresolvable uses in an unguarded job, timeout/cancel, un-guarded job) as the fleet-registration must-fixes, plus break-glass owner.
📝 WalkthroughWalkthroughThis PR adds a new design document reporting results of a SAST gate canary experiment. It documents test beds evaluating whether required-workflow gates block merges based on workflow run conclusion versus per-job status, includes a decision table, break-glass ownership, and cleanup steps. ChangesSAST Gate Canary Documentation
Estimated code review effort: 1 (Trivial) | ~5 minutes Related PRs: None identified. Suggested labels: documentation Suggested reviewers: None identified.
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/design/2026-07-03-fleet-sast-canary-results.md`:
- Around line 72-87: Add the source-repo accessibility failure mode to the
decision table and must-fix summary. In the markdown section containing the
decision table and “Bottom line,” update the fleet-registration blocker list to
include an inaccessible required-workflow source repo as a merge blocker, and
note the recovery action for the workflow header/required-workflow setup so
operators know how to remediate it. Use the existing decision table wording and
the bottom-line “MUST-FIXes” list as the place to insert this new entry.
- Around line 89-91: The break-glass owner wording is too broad and should be
replaced with a specific named owner plus explicit rollback trigger and speed
target. Update the “Break-glass owner” section in the fleet-sast canary design
doc to identify the accountable person or team contact, describe the exact
condition that allows pulling `sast.yaml` from ruleset `#126206`, and state the
expected time-to-unblock so the rollback path is unambiguous.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 6943df7f-a86a-4e7c-959c-6edbfdfd67c5
📒 Files selected for processing (1)
docs/design/2026-07-03-fleet-sast-canary-results.md
📜 Review details
🧰 Additional context used
🪛 markdownlint-cli2 (0.22.1)
docs/design/2026-07-03-fleet-sast-canary-results.md
[warning] 73-73: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
Comment on lines
+72
to
+87
| ## Decision table (failure mode -> run conclusion -> blocks merge? -> mitigation) | ||
| | Failure mode | Run conclusion | Blocks merge? | Mitigation before fleet registration | | ||
| |---|---|---|---| | ||
| | scanner finding (semgrep exits 0) | success | NO | none -- job stays green | | ||
| | step/job crash inside a guarded job (pipx/network/bad-pin) | success | NO (proven C2) | none -- continue-on-error absorbs it | | ||
| | YAML/syntax error in sast.yaml | failure (startup) | YES | pre-merge actionlint/yamllint gate on github-required-workflows | | ||
| | unresolvable `uses:` in an UN-guarded job | failure | YES | actionlint pin check; keep uses inside guarded jobs | | ||
| | timeout | cancelled | YES (run != success) | set an acceptable timeout-minutes; treat cancel as don't-block if feasible | | ||
| | cancellation | cancelled | YES (run != success) | operational; rare for a required run | | ||
| | job WITHOUT continue-on-error fails | failure | YES | lint asserting every job in sast.yaml carries continue-on-error: true | | ||
|
|
||
| Bottom line: blocking is driven purely by RUN conclusion. continue-on-error | ||
| makes the run conclude success for any step/job failure it can reach (findings | ||
| AND crashes). The residual blockers are the modes the guard cannot cover: | ||
| unparseable YAML, timeout/cancel, and any job that lacks the guard. Those are | ||
| the fleet-registration MUST-FIXes above. |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Add the source-repo accessibility blocker to the must-fix list.
This checklist is not exhaustive yet: the workflow header already treats an inaccessible required-workflow source repo as a fleet-wide merge blocker. Since this section is the pre-registration blocker set, please add that failure mode and recovery action so operators do not miss it.
Proposed doc update
| timeout | cancelled | YES | set an acceptable timeout-minutes; treat cancel as don't-block if feasible |
| cancellation | cancelled | YES | operational; rare for a required run |
| job WITHOUT continue-on-error fails | failure | YES | lint asserting every job in sast.yaml carries continue-on-error: true |
+| inaccessible required-workflow source repo | failure | YES | restore source availability / re-point the required workflow before retrying |📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| ## Decision table (failure mode -> run conclusion -> blocks merge? -> mitigation) | |
| | Failure mode | Run conclusion | Blocks merge? | Mitigation before fleet registration | | |
| |---|---|---|---| | |
| | scanner finding (semgrep exits 0) | success | NO | none -- job stays green | | |
| | step/job crash inside a guarded job (pipx/network/bad-pin) | success | NO (proven C2) | none -- continue-on-error absorbs it | | |
| | YAML/syntax error in sast.yaml | failure (startup) | YES | pre-merge actionlint/yamllint gate on github-required-workflows | | |
| | unresolvable `uses:` in an UN-guarded job | failure | YES | actionlint pin check; keep uses inside guarded jobs | | |
| | timeout | cancelled | YES (run != success) | set an acceptable timeout-minutes; treat cancel as don't-block if feasible | | |
| | cancellation | cancelled | YES (run != success) | operational; rare for a required run | | |
| | job WITHOUT continue-on-error fails | failure | YES | lint asserting every job in sast.yaml carries continue-on-error: true | | |
| Bottom line: blocking is driven purely by RUN conclusion. continue-on-error | |
| makes the run conclude success for any step/job failure it can reach (findings | |
| AND crashes). The residual blockers are the modes the guard cannot cover: | |
| unparseable YAML, timeout/cancel, and any job that lacks the guard. Those are | |
| the fleet-registration MUST-FIXes above. | |
| ## Decision table (failure mode -> run conclusion -> blocks merge? -> mitigation) | |
| | Failure mode | Run conclusion | Blocks merge? | Mitigation before fleet registration | | |
| |---|---|---|---| | |
| | scanner finding (semgrep exits 0) | success | NO | none -- job stays green | | |
| | step/job crash inside a guarded job (pipx/network/bad-pin) | success | NO (proven C2) | none -- continue-on-error absorbs it | | |
| | YAML/syntax error in sast.yaml | failure (startup) | YES | pre-merge actionlint/yamllint gate on github-required-workflows | | |
| | unresolvable `uses:` in an UN-guarded job | failure | YES | actionlint pin check; keep uses inside guarded jobs | | |
| | timeout | cancelled | YES (run != success) | set an acceptable timeout-minutes; treat cancel as don't-block if feasible | | |
| | cancellation | cancelled | YES (run != success) | operational; rare for a required run | | |
| | job WITHOUT continue-on-error fails | failure | YES | lint asserting every job in sast.yaml carries continue-on-error: true | | |
| | inaccessible required-workflow source repo | failure | YES | restore source availability / re-point the required workflow before retrying | | |
| Bottom line: blocking is driven purely by RUN conclusion. continue-on-error | |
| makes the run conclude success for any step/job failure it can reach (findings | |
| AND crashes). The residual blockers are the modes the guard cannot cover: | |
| unparseable YAML, timeout/cancel, and any job that lacks the guard. Those are | |
| the fleet-registration MUST-FIXes above. |
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 73-73: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/design/2026-07-03-fleet-sast-canary-results.md` around lines 72 - 87,
Add the source-repo accessibility failure mode to the decision table and
must-fix summary. In the markdown section containing the decision table and
“Bottom line,” update the fleet-registration blocker list to include an
inaccessible required-workflow source repo as a merge blocker, and note the
recovery action for the workflow header/required-workflow setup so operators
know how to remediate it. Use the existing decision table wording and the
bottom-line “MUST-FIXes” list as the place to insert this new entry.
Comment on lines
+89
to
+91
| ## Break-glass owner (decided) | ||
| Anyone on `@tatari-tv/security` or `@tatari-tv/sre` may pull `sast.yaml` from | ||
| ruleset #126206 if it jams merges. |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Make the break-glass owner specific.
Anyone on @tatari-tv/securityor@tatari-tv/sre`` is too broad for the documented rollback contract. The upstream design calls for a named owner plus a rollback trigger and speed target; please record those explicitly so the unblock path is unambiguous.
Proposed doc update
-Anyone on `@tatari-tv/security` or `@tatari-tv/sre` may pull `sast.yaml` from
-ruleset `#126206` if it jams merges.
+Break-glass owner: <named primary>, <named backup>.
+Trigger: fleet-wide merge block attributable to SAST.
+Target: rollback in <minutes>, not hours.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| ## Break-glass owner (decided) | |
| Anyone on `@tatari-tv/security` or `@tatari-tv/sre` may pull `sast.yaml` from | |
| ruleset #126206 if it jams merges. | |
| ## Break-glass owner (decided) | |
| Break-glass owner: <named primary>, <named backup>. | |
| Trigger: fleet-wide merge block attributable to SAST. | |
| Target: rollback in <minutes>, not hours. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/design/2026-07-03-fleet-sast-canary-results.md` around lines 89 - 91,
The break-glass owner wording is too broad and should be replaced with a
specific named owner plus explicit rollback trigger and speed target. Update the
“Break-glass owner” section in the fleet-sast canary design doc to identify the
accountable person or team contact, describe the exact condition that allows
pulling `sast.yaml` from ruleset `#126206`, and state the expected time-to-unblock
so the rollback path is unambiguous.
dontiveros-tatari
approved these changes
Jul 5, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds the evidence companion to
docs/design/2026-07-03-fleet-sast.md(already on main via #40).Records the SAST gate canary that resolved the crux question before fleet registration:
tatari-tv/sast-canary) — the decisive live-gate test.continue-on-errormakes the run concludesuccessfor findings AND guarded crashes, so those do not block merges.sast.yamlin ruleset #126206): unparseable YAML, unresolvableuses:in an un-guarded job, timeout/cancel, and any job missing the guard.Full narrative also published to marquee: https://marquee.internal.tatari.dev/p/~scott-idler/sast-gate-canary-results
Doc-only change; no workflow/code changes.