fix(spine): the platform's only CRITICAL self-finding was permanent and benign #5495
ccantynzAI Reviewcommented 20d ago
AI Triage
(no summary)
Priority: medium Risk area: mixed
Suggested labels: (no label suggestions) Suggested reviewers: (no reviewer suggestions)
Suggestions only — nothing has been applied. The PR author stays in control.
gluecron[bot]🤖 botAI Reviewcommented 20d ago
AI review unavailable
The platform's AI balance is exhausted, so AI generation is temporarily unavailable. Nothing was lost. You can queue this as a repair for the internal agent from the repository's Health page, or try again once the balance is restored. The PR is otherwise unchanged.
Cross-repo impact
See what breaks downstream if this PR merges.
⮌ Merged
This pull request was merged into main.
c comment · e edit title · m merge · a approve · r request changes · ? shortcuts
The finding
bun scripts/selfcheck.tsreportedcritical=1on every run, holding the self-score at 75/100. Production logged it every tick:It is
raw-sql-interpolationatspine-alert-fanout.ts:123:interval '${sql.raw(String(SPIKE_WINDOW_MINUTES))} minutes'Why it was never a vulnerability — and why it still mattered
SPIKE_WINDOW_MINUTESis a module constant (export const SPIKE_WINDOW_MINUTES = 10), never user input. There is no injection path.But
new=0means it had been reported unchanged for as long as the rule has existed, and that is the damage. A CRITICAL that is permanently red and permanently harmless is worse than no signal at all: it caps the score, it fires an alert line every tick, and it teaches every reader to scroll past the one severity the rule exists to make them stop at. The next genuinely-injectable interpolation would have landed in a channel nobody reads.The fix
now() - make_interval(mins => ${SPIKE_WINDOW_MINUTES})Binds the window as a query parameter and removes
sql.rawfrom the path entirely. Same semantics, same plan shape.Cleared by construction, not by a
selfcheck-ignoremarker. A suppression would leave the rule blind to a real interpolation appearing on this line later — and selfcheck already reports twodead-suppressionfindings (github-issue-import.ts:526,628), which is the same mistake one step further along: a marker that outlived its finding and now reads as a considered decision.Verification
spine-alert-fanout+admin-spine: 11 pass.tsc --noEmitclean.Remaining selfcheck findings (not in this PR)
high 2—gate.ts:566unvalidated-git-ref,synthetic-journeys.ts:189swallowed-rejection. Both worth their own look; the git-ref one especially.