diff --git a/scripts/frozen-weights-check.mjs b/scripts/frozen-weights-check.mjs index 767db378a..7a678e0ad 100644 --- a/scripts/frozen-weights-check.mjs +++ b/scripts/frozen-weights-check.mjs @@ -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');