fix(frozen-weights-check): regex-literal-aware stripping closes JS false-negative (PR #869 re-verify)

The re-verify found a residual false-NEGATIVE the prior fix mis-documented
as "false-positive-direction only": the char scanner had no regex-literal
awareness, so a `//` inside a JS regex was read as a line comment and ate a
trailing denied token. Two live-code PoCs passed silently:
  const re = /[a//]/; const w = "evil-model.gguf";   // .gguf was stripped
  if (/https?:\/\//.test(u)) loadModel("x.gguf")      // .gguf was stripped

Fix: add minimal regex-literal recognition to the JS branch. A single `/`
(never `//` or `/*`, which stay always-comments — an empty `//` regex is
illegal JS) opens a regex when it appears in VALUE position; in operator
position it is division. Value/operator is tracked by the last significant
token (identifiers/numbers/`)`/`]`/closed string|regex|template => operand =>
division; `=` `(` `,` `[` `{` `;` `:` operators and value-keywords like
return/typeof/new/throw => value => regex). A regex body — including internal
`//` and `[...]` char classes (where `/` is literal) — is preserved as
content and never comment-stripped. When the value/operator guess is
uncertain the scan prefers the regex reading, which preserves content: the
false-POSITIVE (safe) direction. The header's residual note is corrected
accordingly — the JS-regex false-negative is closed, not merely documented.

Self-test gains both PoCs (must flag), the ordinary-code guard form
`if (/https?:\/\//.test(u)) loadModel("x.gguf")` (must flag), a
division-then-real-comment case and a regex-then-real-comment case (comment
tokens must NOT flag). Stress-probed 11 further variants (paren-division,
regex in array/after-comma/ternary-branch, flags, nested-class slashes,
assign-division chains): every denied token in code/strings preserved, only
denied tokens inside genuine trailing comments stripped.

Self-test: 27/27 assertions pass. Real-tree scan: 25 files, 0 hits.
Harness suite: 26/26 green.

Refs #841, PR #869 re-verify.

Co-Authored-By: claude-flow <ruv@ruv.net>
Claude-Session: https://claude.ai/code/session_012Jib2gQyJpqCoo2xYAbb4X
This commit is contained in:
ruv 2026-08-20 09:23:03 -04:00
parent 677490aae3
commit ae7d875cbc

View file

@ -35,14 +35,18 @@
* the dishonest path deliberate. Comments are stripped before matching so
* that PROSE about training (e.g. darwin_guard.rs's train/eval-contamination
* doc comments) does not false-positive only code and string literals count.
* The stripper is STRING-AWARE (PR #869 security audit, MEDIUM finding): it
* is a per-language character scan that tracks quote state, so a `//` inside
* a string literal "https://hf.co/repo/model.safetensors" is content, not
* a comment, and denied tokens in URLs are caught. Residual stripper edges
* (all in the false-POSITIVE direction, never hiding a token): a JS regex
* literal containing `//` may over-strip the rest of its line; a Rust char
* literal containing '"' or a raw string r#"…"# may leave a comment
* unstripped. Template literals are tracked including ${} nesting.
* The stripper is STRING-AWARE (PR #869 audit, MEDIUM finding): it is a
* per-language character scan that tracks quote state, so a `//` inside a
* string literal "https://hf.co/repo/model.safetensors" is content, not a
* comment, and denied tokens in URLs are caught. It is also REGEX-AWARE (PR
* #869 re-verify): a JS regex literal (`/…/`) has its interior preserved, so
* a `//` or `[…]` inside a regex no longer eats a denied token later on the
* line. Template literals are tracked including ${} nesting. The remaining
* stripper edges are all in the false-POSITIVE (over-flag / safe) direction,
* never hiding a denied token: a Rust char literal containing '"' or a raw
* string r#"…"# may leave a comment unstripped, and a JS division mis-guessed
* as a regex preserves its body as content. See stripComments() for the full
* per-language contract and the residual-edge note.
*
* Accepted residuals (on record from the PR #869 audit, not closed here):
* - `fixtures/` and `node_modules/` under a surface stay unscanned by
@ -172,13 +176,30 @@ function warn(msg) {
* Per-language rules:
* - js-like (.ts/.mts/.cts/.js/.mjs/.cjs): `//` + `/* */` comments;
* `'`, `"`, and template-literal strings, with `${}` re-entry tracked
* (nested braces counted) and backslash escapes honored.
* (nested braces counted) and backslash escapes honored; and REGEX
* LITERALS (PR #869 re-verify): a `/` in value position opens a regex
* whose interior including any `//` and `[...]` char classes is
* content, never comment-stripped, so a denied token sharing a line with
* a regex is not silently dropped. `//` and `/* */` are still always
* comments (an empty `//` regex is illegal JS), so comment recognition
* wins first; only a single `/` not followed by `/` or `*` can open a
* regex. Value vs. operator (division) position is tracked by the last
* significant token; when uncertain the scan prefers the regex reading,
* which preserves content the FALSE-POSITIVE (safe) direction.
* - rust (.rs): `//` + NESTED `/* */` comments; `"` strings only `'` is
* deliberately not a string delimiter (lifetimes like `&'a str` would
* desynchronize the scan; char literals cannot hide a multi-char token).
* - python (.py): `#` comments; `'`/`"` and triple-quoted strings.
* - shell (.sh): `#` comments (only at line start or after whitespace, so
* `$#`/`${#x}` survive); `'` (no escapes) and `"` strings.
*
* Residual stripper edges, ALL in the false-POSITIVE (over-flag) direction
* they can leave a comment unstripped or preserve extra content, never hide a
* denied token: a Rust char literal containing `"` or a raw string `r#"…"#`;
* a js division mis-read as a regex when the value/operator heuristic guesses
* wrong (preserves the "regex" body as content). The JS-regex FALSE-NEGATIVE
* the PR #869 re-verify found (a regex's internal `//` eating a trailing
* token) is closed by the regex-literal handling above.
*/
function stripComments(source, filename) {
const lang = filename.endsWith('.rs') ? 'rs'
@ -187,12 +208,23 @@ function stripComments(source, filename) {
: 'js';
const slashComments = lang === 'js' || lang === 'rs';
const hashComments = lang === 'py' || lang === 'sh';
// Keywords after which a `/` still begins a VALUE (regex), not division.
const VALUE_KEYWORDS = new Set([
'return', 'typeof', 'instanceof', 'in', 'of', 'new', 'delete', 'void',
'throw', 'else', 'do', 'yield', 'await', 'case',
]);
const out = [];
// For js template literals: one entry per open `${`, counting nested braces.
const templateBraces = [];
let mode = 'code'; // 'code' | 'line' | 'block' | 'string'
let mode = 'code'; // 'code' | 'line' | 'block' | 'string' | 'regex'
let blockDepth = 0;
let quote = ''; // ' " ` or ''' """ while mode === 'string'
let regexInClass = false; // inside [...] while mode === 'regex'
// js only: does the next `/` divide (true) or open a regex (false)?
// Starts false — a `/` at the very start of a file opens a regex, not a
// comment/division. Only a clear operand (identifier, number, `)`, `]`,
// or a closed string/regex/template) sets it true.
let expectOperand = false;
let i = 0;
while (i < source.length) {
const ch = source[i];
@ -209,27 +241,47 @@ function stripComments(source, filename) {
}
out.push(ch === '\n' ? '\n' : ' '); i += 1; continue;
}
if (mode === 'regex') {
// Content-preserving: emit everything; a `//` or `[...]` here is regex
// syntax, not a comment. Bail on a raw newline (unterminated regex).
if (ch === '\\' && i + 1 < source.length) { out.push(source.slice(i, i + 2)); i += 2; continue; }
if (ch === '\n') { out.push('\n'); mode = 'code'; expectOperand = false; i += 1; continue; }
if (ch === '[') regexInClass = true;
else if (ch === ']') regexInClass = false;
else if (ch === '/' && !regexInClass) {
out.push('/'); i += 1;
while (i < source.length && /[a-z]/i.test(source[i])) { out.push(source[i]); i += 1; } // flags
mode = 'code'; expectOperand = true; continue;
}
out.push(ch); i += 1; continue;
}
if (mode === 'string') {
const noEscapes = quote.length === 3 || (lang === 'sh' && quote === "'");
if (!noEscapes && ch === '\\') { out.push(source.slice(i, i + 2)); i += 2; continue; }
if (quote === '`' && source.startsWith('${', i)) {
templateBraces.push(0); mode = 'code'; out.push('${'); i += 2; continue;
templateBraces.push(0); mode = 'code'; expectOperand = false; out.push('${'); i += 2; continue;
}
if (quote.length === 3 ? source.startsWith(quote, i) : ch === quote) {
out.push(quote); mode = 'code'; i += quote.length; continue;
out.push(quote); mode = 'code'; expectOperand = true; i += quote.length; continue;
}
out.push(ch); i += 1; continue;
}
// mode === 'code'
if (templateBraces.length > 0 && (ch === '{' || ch === '}')) {
const top = templateBraces.length - 1;
if (ch === '{') templateBraces[top] += 1;
if (ch === '{') { templateBraces[top] += 1; expectOperand = false; }
else if (templateBraces[top] === 0) { templateBraces.pop(); mode = 'string'; quote = '`'; }
else templateBraces[top] -= 1;
else { templateBraces[top] -= 1; }
out.push(ch); i += 1; continue;
}
if (slashComments && source.startsWith('//', i)) { mode = 'line'; out.push(' '); i += 2; continue; }
if (slashComments && source.startsWith('/*', i)) { mode = 'block'; blockDepth = 1; out.push(' '); i += 2; continue; }
// js regex-vs-division: a single `/` (not // or /*) in value position
// opens a regex literal; in operator position it is division.
if (lang === 'js' && ch === '/') {
if (expectOperand) { out.push('/'); expectOperand = false; i += 1; continue; } // division
mode = 'regex'; regexInClass = false; out.push('/'); i += 1; continue;
}
if (hashComments && ch === '#' && (lang === 'py' || i === 0 || /\s/.test(source[i - 1]))) {
mode = 'line'; out.push(' '); i += 1; continue;
}
@ -239,6 +291,24 @@ function stripComments(source, filename) {
if (ch === '"' || (ch === "'" && lang !== 'rs') || (ch === '`' && lang === 'js')) {
quote = ch; mode = 'string'; out.push(ch); i += 1; continue;
}
// js value/operator tracking for the regex heuristic.
if (lang === 'js') {
if (/[A-Za-z0-9_$]/.test(ch)) {
let j = i + 1;
while (j < source.length && /[A-Za-z0-9_$]/.test(source[j])) j += 1;
const word = source.slice(i, j);
out.push(word);
// A number or non-value-keyword identifier is an operand; a
// value-keyword (return, typeof, …) still expects a value next.
expectOperand = !VALUE_KEYWORDS.has(word);
i = j; continue;
}
if (!/\s/.test(ch)) {
// Punctuation: `)` and `]` close an operand; everything else
// (`(`, `,`, `=`, `;`, `:`, `{`, operators …) expects a value next.
expectOperand = ch === ')' || ch === ']';
}
}
out.push(ch); i += 1;
}
return out.join('');
@ -396,10 +466,13 @@ function check(repoRoot = REPO_ROOT) {
* 2. training-import / MCP-tool / model-file violations each fail with the
* right deny-list id including URL-form references
* ("https://…/model.safetensors", "file://…/x.gguf", "http://…/finetune",
* the PR #869 MEDIUM), the single-slash "ruvllm/training" import form,
* .py/.sh helpers, and files under a committed dist/;
* 3. comment-only mentions of denied tokens do NOT fail (plain comments and
* comments containing URLs alike);
* the PR #869 MEDIUM), regex literals whose internal `//` must not eat a
* trailing denied token (PR #869 re-verify PoCs), the single-slash
* "ruvllm/training" import form, .py/.sh helpers, and files under a
* committed dist/;
* 3. comment-only mentions of denied tokens do NOT fail (plain comments,
* comments containing URLs, division-then-comment, and
* regex-then-comment lines alike);
* 4. symlinks (including one pointing at an out-of-tree file full of
* violations) are skipped with a warning, never followed, no stack trace;
* 5. a deleted mutation surface fails loudly (missing-surface hardening).
@ -443,6 +516,14 @@ function selfTest() {
// Comment containing a URL with a denied token: still a comment, still safe.
writeFileSync(join(harnessSrc, 'comment-url.ts'),
'// background reading: http://internal/finetune docs\nexport const y = 2;\n');
// True division then a real trailing comment: the comment (with its denied
// token) must still strip — division must not derail comment recognition.
writeFileSync(join(harnessSrc, 'division-comment.ts'),
'const q = a / b / c; // note: models/x.gguf\nexport {q};\n');
// Regex literal then a real trailing comment: the comment's denied token
// must NOT be flagged (the regex closes; `//` after it is a comment).
writeFileSync(join(harnessSrc, 'regex-comment.ts'),
'const re = /ab+c/; // mentions model.safetensors in prose\nexport {re};\n');
writeFileSync(join(mragent, 'clean.sh'),
'#!/bin/sh\n# fine_tune mentioned only in this comment\necho ok\n');
writeFileSync(join(root, 'outside-violations.rs'),
@ -472,6 +553,15 @@ function selfTest() {
// Single-slash module path must keep flagging (auditor's regression case).
writeFileSync(join(harnessSrc, 'bad-import-slash.ts'),
'import { t } from "ruvllm/training";\n');
// PR #869 re-verify: a `//` inside a JS regex must NOT eat the trailing
// denied token on the same line. Both PoCs are live JS.
writeFileSync(join(harnessSrc, 'bad-regex-class.ts'),
'const re = /[a//]/; const w = "evil-model.gguf"; export {re, w};\n');
writeFileSync(join(harnessSrc, 'bad-regex-escaped.ts'),
'const re = /https:\\/\\//; loadWeights("m.safetensors"); export {re};\n');
// Ordinary-code form: regex guard then a real model load on the same line.
writeFileSync(join(harnessSrc, 'bad-regex-guard.ts'),
'if (/https?:\\/\\//.test(u)) loadModel("x.gguf");\n');
// New coverage: .py / .sh helpers and committed dist/ output are scanned.
writeFileSync(join(mragent, 'bad.py'),
'trainer.save_checkpoint("out") # totally routine\n');
@ -495,6 +585,12 @@ function selfTest() {
'http URL to /finetune is flagged (string-aware stripper)');
ok(dirty.stderr.includes('bad-import-slash.ts'),
'single-slash ruvllm/training import still flags');
ok(dirty.stderr.includes('bad-regex-class.ts') && dirty.stderr.includes('model-file-reference'),
'regex with // in a char class does NOT eat the trailing .gguf (PoC a)');
ok(dirty.stderr.includes('bad-regex-escaped.ts'),
'regex with escaped // does NOT eat the trailing .safetensors (PoC b)');
ok(dirty.stderr.includes('bad-regex-guard.ts'),
'regex guard then loadModel(".gguf") on one line is flagged (ordinary code)');
ok(dirty.stderr.includes('bad.py') && dirty.stderr.includes('weight-writing'),
'.py helper invoking a weight writer is flagged');
ok(dirty.stderr.includes('bad.sh') && dirty.stderr.includes('generic-finetune'),
@ -506,6 +602,10 @@ function selfTest() {
'denied token in a genuine comment URL is NOT flagged');
ok(!dirty.stderr.includes('clean.sh'),
'.sh with denied token only in a comment is NOT flagged');
ok(!dirty.stderr.includes('division-comment.ts'),
'division then a real comment: comment token still stripped (not flagged)');
ok(!dirty.stderr.includes('regex-comment.ts'),
'regex then a real comment: comment token NOT flagged');
ok(!dirty.stderr.includes('linked.rs: ['), 'symlinked violations are NOT followed/flagged');
ok(!dirty.stderr.includes('darwin_guard.rs'),
'train/eval-contamination guard prose does NOT false-positive');