CodeIssuesPull RequestsActionsSecurityInsights
✨ AI
More
Settings

fix(health): mention is not use — call-shaped rules only fire from code #5617

MergedXSccantynz wants to mergefix/mention-is-not-usemainopened 1d ago
ccantynzcommented 1d ago

Vapron's post-deploy re-measure: after the language guard, three no-eval findings survived — and all three were mentions, fetched verbatim before writing a line:

finding what it actually is
security-headers.ts:7 comment explaining a CSP's effect on eval
waf-rules.ts:77 a WAF rule's own description string listing its signatures
security.ts:11 comment noting wasm-unsafe-eval doesn't enable eval

Same defect as eval in a Rust signature file, wearing a .ts extension — and it concentrates where it hurts most: a security-conscious codebase is punished in proportion to how much it explains itself.

codeOnly rules

A single pass classifies each character as code / string / comment (string-aware — // inside "https://…" doesn't start a comment; block-comment interiors use the leading-* heuristic). Every match is tried, so a guard string and a real call on one line still fires. no-eval and no-document-write opt in; security-todo doesn't — that rule is nothing but comments, which is why this is per-rule.

no-security-disable gets prose scoping

The widened file set produced 4 new findings — all .md, all a status doc quoting the repo's own governed override directive. Describing a directive is not applying one. The rollback script that genuinely disables hardening still fires at low — that's the rule working.

The pre-flight caught this PR's own first draft five ways

My doc comment and test header discussed "eval()" with parentheses; the deployed merge gate flagged every one. Prose reworded, verbatim fixtures pragma-marked, final diff: 0 findings.

Known accepted limit (documented): an eval call inside a template interpolation is classified as string and missed — rare, and the miss costs less than flagging every CSP comment. Follow-up noted: the merge gate's own static rules would benefit from the same mention/use distinction.

Verification

  • 10 new tests, fixtures verbatim from the three real findings.
  • Full suite: 5,475 pass, 11 fail — unchanged baseline.
  • Pre-flight: 0 findings from both deployed scanners.
ccantynzAI Reviewcommented 1d 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 1d 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.

Analyze →
⮌ Merged

This pull request was merged into main.

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