CodeIssuesPull RequestsActionsSecurityInsights
✨ AI
More
Settings

Visual audit: switched-off scanners, dead-end pages, and a container with no memory limit #5562

Merged⚡ AI-generatedXSccantynz wants to mergefix/reinstate-raw-sql-scannermainopened 4d ago
ccantynzcommented 4d ago

Ten commits from the first session with a browser actually attached. Most of these were invisible from the source and obvious in one screenshot.

CI: 4876 pass, 0 fail, no error line.

Two scanners had been switched off to keep the build green

22 of 395 test files are excluded from CI, listed as "wrong environment, needs an audit". Two audited:

  • raw-sql-rule — not env-sensitive. Simply red, for one finding: truth-ledger.ts calls sql.raw(). Adjudicated a false positive (every caller passes a compile-time literal from that file's own claim table) and suppressed inline with that reason.
  • json-in-script — same story, one site. Fixed rather than suppressed: jsonForScript already existed and swapping it in was one word, where a suppression would have carved a permanent hole in an XSS guard to save the same keystroke.

What the exclusions actually cost: these rules detect raw SQL splicing and script-tag-closing values anywhere in the tree. Excluding them to go green disabled that protection across all 395 files to avoid adjudicating two lines. An exclusion that silences a scanner is not the same as one that skips a test, and the excludes file now says so.

Security scanning no longer depends on the AI bill

The "Security scan" gate was AI-only. When the platform's Anthropic balance ran out it reported "AI security scan unavailable — scan skipped" on every push: a billing condition had become a security-coverage condition, for every customer at once.

The Secret scan beside it never stopped — 15 regexes, no credit. That's the proof the dependency was incidental.

STATIC_SECURITY_RULES adds eight high-signal rules running on every push regardless: SQL/command injection by interpolation, eval/new Function, raw HTML injection, JSON.stringify in inline scripts, broken hashes, disabled TLS verification, non-timing-safe secret comparison.

Deliberate limits so it's a floor, not a false ceiling: scans the diff (repos are bare), added lines only (flagging what someone is deleting teaches people to ignore the scanner), high-signal only.

⚠️ Behaviour change: a critical static finding now fails the gate even while AI is down. The old test asserted the opposite and its fixture was literally + eval(userInput) — it passed only because an outage meant nothing inspected the diff.

Also: gate.ts rendered the provider's raw JSON envelope to users, twice per page, leaking an internal request_id. humanizeAiError() existed for exactly this and was never called.

Pages that were dead ends or shouting

  • A vertical scrollbar inside the horizontal repo tab row, every repo page. overflow-x:auto forces overflow-y:auto; margin-bottom:-1px gave it 1px to scroll; the webkit rule zeroed height but never width.
  • Deep pages lost the repo identity card and spent the space restating the page name three times, pushing real content to ~half the viewport. All heroes across Insights, Actions and Pull Requests compacted; the time-travel page kept its information (file path, revisions) because that's data, not a nameplate.
  • The repo list was a rainbow — 45 saturated bars in four hues plus matching grade letters. Now a hairline and one chip. Removed 180 identical links (Health/Deps/Insights/Settings × 45). Added a filter with match count, and favourites-first ordering reusing the existing stars table.

Infrastructure

  • Container had HostConfig.Memory=0 and leaked, taking a shared host to 96% full via five 10–13 GB core dumps. Bounded (mem_limit, ulimits.core: 0). The autohealer beside it had been silently restarting it for three days — an autohealer plus no limit turns a visible crash into an invisible cycle. The leak itself is not fixed; this bounds the blast radius.
  • Imports now record their upstream instead of silently rotting. upsertMirror() existed and the import path never called it, which is why a repo could describe itself as a mirror while syncing from nothing since July.

The thread through all of it

A thing, and a description of the thing, drifted apart — and the description won because nothing checked. A scanner reporting on a surface it stopped reading. A mirror description with no mirror row. A monitor that existed only in a comment. An off-switch that governs nothing. A catch that can't fire.

🤖 Generated with Claude Code

gluecron[bot]🤖 botAI Reviewcommented 4d 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.

ccantynzAI Reviewcommented 4d 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.

Cross-repo impact

See what breaks downstream if this PR merges.

Analyze →
⮌ Merged

This pull request was merged into main.

c comment · e edit title · m merge · a approve · r request changes · ? shortcuts