fix: the cross-engine diff's findings — one real eval, one self-portrait #5619
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.
⮌ Merged
This pull request was merged into main.
c comment · e edit title · m merge · a approve · r request changes · ? shortcuts
GateTest ran its full suite against
e168803. Two engines over one tree found what neither found alone — in both directions.The real one: the only live
evalin this repositoryscripts/interaction-audit.mjspassed a 44-line check function into the page as a source string and raneval(checkSrc)(panel).Both engines had missed it: theirs because 21 call sites collected
.js/.ts/.jsx/.tsxand never opened a.mjs; ours because nobody read our own report. Precision on files you never open is not precision — their words, their miss, our file.The string had no reason to exist —
evaluate()serialises its callback anyway. The check is now a real function written where it runs.evalis gone from the repo.The self-portrait: the scanner reporting its own safety predicate
All four remaining innerHTML findings on this tree were comments inside
innerHtmlAssignmentIsSafe's own documentation. The rule was reporting the documentation of its own fix.Fixed with the weaker exclusion (
notInComments), notcodeOnly, and the distinction is load-bearing: an admin page served as a template literal carries real innerHTML assignments inside a string — the ops-dashboard class this rule was tuned on — so string interiors stay eligible while comments don't. Tests pin both directions.Their other two reports, triaged with evidence
intelligence.ts"shell exec with interpolated input" — their FP: the site isexec(["git","show",…], dir), an argv array. No shell parses the value;exec()centrally refuses option-like args. Reported back with the mechanism.first-run-journey.mjsMath.random password — real enough to fix: the journey creates a real account on production, and timestamp+Math.random is guessable by anyone with a clock. Nowcrypto.randomBytes. Throwaway is a policy, not a defence.Verification
node --checkon both scripts; pre-flight 0 findings; full suite 5,484 pass / 11 fail baseline (+ known flaky).