From 6ee7ce1cc463a8ad55cbdf2aea8325e0eb0396e0 Mon Sep 17 00:00:00 2001 From: Gergo Magyar Date: Sat, 24 Oct 2026 09:16:37 +0000 Subject: [PATCH] fix(taint): preserve guard proof identities Reject destructured/rest formals before they inherit a whole-argument proof, and include transitive alias uses in bounded guard certificates. Add worker regressions, argument-heavy scaling coverage and global refinement fallback diagnostics. Refs #3703 --- gitnexus/bench/cfg/baselines.json | 10 +++ gitnexus/bench/cfg/measure.mjs | 10 +++ .../cfg/visitors/typescript-harvest.ts | 3 + .../pipeline-phases/taint-summaries.ts | 5 ++ .../core/ingestion/taint/guard-refinement.ts | 46 ++++++++++---- .../test/integration/cfg/guard-taint.test.ts | 63 +++++++++++++++++++ .../unit/cfg/typescript-refinement.test.ts | 16 ++++- .../test/unit/taint/guard-refinement.test.ts | 14 +++++ gitnexus/test/unit/taint/propagate.test.ts | 30 +++++++++ 9 files changed, 185 insertions(+), 12 deletions(-) diff --git a/gitnexus/bench/cfg/baselines.json b/gitnexus/bench/cfg/baselines.json index fa172726f..dbcacad35 100644 --- a/gitnexus/bench/cfg/baselines.json +++ b/gitnexus/bench/cfg/baselines.json @@ -1,4 +1,14 @@ { + "guard-free-arguments": { + "fingerprint": "819f475c96ced034846e1c7d8aa08613386b258f6df276a1e4058944fffa9231", + "scaling_budget": 1.8, + "disk_bytes_budget": 1.2, + "heap_budget": 1.3, + "rd_scaling_budget": 2.0, + "facts_large_min": 8000, + "disk_bytes_large_max": 6000000, + "_note": "#3703 review: 1000/4000 guard-free functions with const composition and argument forwarding exercise value-fact capture even without predicates. Initial normalized time/disk/heap/def-use ratios: 0.878/1.012/0.999/0.834; large payload 5,306,959 bytes. A separate five-pair comparison at the reviewed head versus the merge base measured +11.26% collection time and +27.0% serialized bytes on the same argument-heavy shape (different file/spacing). This exceeds the plan's proposed 5% guard-free target; the earlier ~1% samples do not cover this capture path. Keep the cost explicit; this gate checks scaling and payload growth, not a validated <=5% overhead claim. Existing scenario budgets remain unchanged." + }, "straight-line": { "fingerprint": "792229965a726d2c6b527f9ee65440a2b3023839ee71cb51522fc30e2f2cb454", "scaling_budget": 1.5, diff --git a/gitnexus/bench/cfg/measure.mjs b/gitnexus/bench/cfg/measure.mjs index 800ea9d26..54f73dcc7 100644 --- a/gitnexus/bench/cfg/measure.mjs +++ b/gitnexus/bench/cfg/measure.mjs @@ -116,6 +116,16 @@ function langToolkit(langId) { // ---- synthetic generators (one cost dimension each) ---- const SCENARIOS = [ + { + name: 'guard-free-arguments', + small: 1000, + large: 4000, + gen: (n) => + Array.from( + { length: n }, + (_, i) => `function f${i}(x: string) { const y = 'prefix ' + x; return consume${i}(y); }`, + ).join('\n'), + }, { name: 'guarded-commands', small: 60, diff --git a/gitnexus/src/core/ingestion/cfg/visitors/typescript-harvest.ts b/gitnexus/src/core/ingestion/cfg/visitors/typescript-harvest.ts index 00861a20d..f4f91ea85 100644 --- a/gitnexus/src/core/ingestion/cfg/visitors/typescript-harvest.ts +++ b/gitnexus/src/core/ingestion/cfg/visitors/typescript-harvest.ts @@ -290,6 +290,9 @@ export class TsHarvester { // `pattern`); plain JS puts the pattern directly in formal_parameters. const pattern = p.childForFieldName('pattern') ?? p; this.declarePattern(pattern, 'param', this.root, true, formalIndex); + // A formal index identifies the argument slot, not a value extracted + // from it. Extraction/default/rest effects are not modeled as identity. + if (pattern.type !== 'identifier') this.refinementSafe = false; if ( this.hasGuardCandidate && pattern.type === 'identifier' && diff --git a/gitnexus/src/core/ingestion/pipeline-phases/taint-summaries.ts b/gitnexus/src/core/ingestion/pipeline-phases/taint-summaries.ts index 96417ff30..1d311cec1 100644 --- a/gitnexus/src/core/ingestion/pipeline-phases/taint-summaries.ts +++ b/gitnexus/src/core/ingestion/pipeline-phases/taint-summaries.ts @@ -85,6 +85,11 @@ export const taintSummariesPhase: PipelinePhase = { const maxEdges = ctx.options?.pdgMaxInterprocEdges ?? DEFAULT_PDG_MAX_INTERPROC_EDGES; const solved = solveInterprocTaint(summaryMap, callEdges, { maxFindings, maxHops }); + if (solved.refinementFallback) { + logger.warn( + '[taint-interproc] refinement memory budget exhausted; resumed conservative analysis', + ); + } const emit = emitInterprocTaint(ctx.graph, solved.findings, { maxEdges }, (m) => logger.warn(m), ); diff --git a/gitnexus/src/core/ingestion/taint/guard-refinement.ts b/gitnexus/src/core/ingestion/taint/guard-refinement.ts index d6294dc2d..eb5be4543 100644 --- a/gitnexus/src/core/ingestion/taint/guard-refinement.ts +++ b/gitnexus/src/core/ingestion/taint/guard-refinement.ts @@ -177,11 +177,16 @@ export function createFunctionRefinement(cfg: FunctionCfg): FunctionRefinement { calls.set(`${block.index}:${index}:${call.site}`, call); } } - const guards = computeGuardRefinements(cfg, { - eligible: cfg.refinementSafe === true, - primitiveBindings: cfg.primitiveBindings ?? [], - predicates, - }); + const guards = computeGuardRefinements( + cfg, + { + eligible: cfg.refinementSafe === true, + primitiveBindings: cfg.primitiveBindings ?? [], + predicates, + }, + undefined, + definitions, + ); const active = cfg.refinementSafe === true && guards.status !== 'fallback'; return { guards, @@ -258,11 +263,16 @@ export function createMemoizedRefinement( }; } -/** Removing an accepted edge must make the use unreachable from ENTRY. */ +/** + * Removing an accepted edge must make the use unreachable from ENTRY. + * Definitions are the validated value expressions reconstructed by the caller. + * Expanding their uses shares the traversal cap; no certificates survive exhaustion. + */ export function computeGuardRefinements( cfg: FunctionCfg, facts?: GuardFacts, limits?: { maxVisits?: number; maxGuards?: number; maxWords?: number }, + definitions?: ReadonlyMap, ): GuardRefinements { if (!facts?.predicates?.length) return NO_GUARDS; let visits = 0; @@ -344,11 +354,25 @@ export function computeGuardRefinements( if (!isInitial) written.add(binding); } for (const binding of statement.uses) { - if (++visits > maxVisits) return fallback(); - if (!subjects.has(binding)) continue; - let blocks = uses.get(binding); - if (!blocks) uses.set(binding, (blocks = new Set())); - blocks.add(block.index); + // Argument reconstruction expands aliases at the sink. Certify the + // subjects reached through those same definitions at that use block. + const pending: ValueExpression[] = [{ kind: 'binding', binding }]; + const seen = new Set(); + while (pending.length) { + if (++visits > maxVisits) return fallback(); + const value = pending.pop()!; + if (value.kind === 'concat') pending.push(...value.parts); + else if (value.kind === 'binding' && !seen.has(value.binding)) { + seen.add(value.binding); + if (subjects.has(value.binding)) { + let blocks = uses.get(value.binding); + if (!blocks) uses.set(value.binding, (blocks = new Set())); + blocks.add(block.index); + } + const definition = definitions?.get(value.binding); + if (definition) pending.push(definition); + } + } } } } diff --git a/gitnexus/test/integration/cfg/guard-taint.test.ts b/gitnexus/test/integration/cfg/guard-taint.test.ts index 385991b3a..f47891651 100644 --- a/gitnexus/test/integration/cfg/guard-taint.test.ts +++ b/gitnexus/test/integration/cfg/guard-taint.test.ts @@ -7,6 +7,69 @@ import { createTempDirPool } from '../../helpers/temp-dir-pool.js'; const repos = createTempDirPool('gn-guard-taint-'); describe('guard-aware command taint (#3490)', () => { + it('preserves alias proofs across blocks and retains destructuring findings in real workers', async () => { + const repo = repos.dir(); + fs.writeFileSync( + path.join(repo, 'aliases.ts'), + ` +import {execSync} from 'node:child_process'; +function indexed({9: command}: any) { execSync(command); } +function defaulted(context: any, {command = context.query.command}: any) { execSync(command); } +function forward(command: string, log: boolean) { + const alias = command; + if (log) console.log('validated'); + execSync(alias); +} +export function handleIndexed(req: any) { + const ref: string = req.query.ref; + if (!/^[a-z]{1,40}$/.test(ref)) return; + indexed('git show ' + ref); +} +export function handleDefaulted(req: any) { + const ref: string = req.query.ref; + if (!/^[a-z]{1,40}$/.test(ref)) return; + defaulted(req, 'git show ' + ref); +} +export function handleAlias(req: any, log: boolean) { + const ref: string = req.query.ref; + if (!/^[0-9a-f]{7,40}$/.test(ref)) return; + const args = 'show ' + ref; + const command = 'git ' + args; + if (log) console.log('validated'); + execSync(command); + forward(command, log); +} +export function handleBypass(req: any, log: boolean) { + const ref: string = req.query.ref; + if (log) { if (!/^[0-9a-f]{7,40}$/.test(ref)) return; } + const command = 'git show ' + ref; + if (log) console.log('unchecked'); + execSync(command); + forward(command, log); +} +`, + ); + const result = await runPipelineFromRepo(repo, () => {}, { pdg: true, workerPoolSize: 1 }); + const paths = [...result.graph.iterRelationships()] + .filter((edge) => edge.type === 'TAINT_PATH') + .map((edge) => result.graph.getNode(edge.sourceId)?.properties.name); + expect(paths).toContain('handleIndexed'); + expect(paths).toContain('handleDefaulted'); + expect(paths).toContain('handleBypass'); + expect(paths).not.toContain('handleAlias'); + const localSources = [...result.graph.iterRelationships()] + .filter((edge) => edge.type === 'TAINTED') + .map((edge) => result.graph.getNode(edge.sourceId)?.properties.startLine); + const source = fs.readFileSync(path.join(repo, 'aliases.ts'), 'utf8').split('\n'); + // TAINTED starts at the request read, immediately after the declaration. + const aliasLine = + source.findIndex((line) => line.startsWith('export function handleAlias')) + 2; + const bypassLine = + source.findIndex((line) => line.startsWith('export function handleBypass')) + 2; + expect(localSources).not.toContain(aliasLine); + expect(localSources).toContain(bypassLine); + }, 120_000); + it('retains findings for bypasses, mutation, uncertain types and unsupported sink uses', async () => { const cases: Record = { bypass: diff --git a/gitnexus/test/unit/cfg/typescript-refinement.test.ts b/gitnexus/test/unit/cfg/typescript-refinement.test.ts index 8545cf2d2..efc072974 100644 --- a/gitnexus/test/unit/cfg/typescript-refinement.test.ts +++ b/gitnexus/test/unit/cfg/typescript-refinement.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from 'vitest'; -import { parse } from '../../helpers/ts-cfg-harness.js'; +import { cfgOf, parse } from '../../helpers/ts-cfg-harness.js'; import { createRefinementFileContext, readPredicate, @@ -38,6 +38,20 @@ function predicate(source: string) { } describe('normalized predicate and expression facts', () => { + it.each([ + '{command}', + '{9: command}', + '[command]', + '[, command]', + '...command', + '{command = fallback()}', + '{nested: {command}}', + ])('keeps non-identity formal bindings conservative: %s', (pattern) => { + const cfg = cfgOf(`function f(${pattern}: any) { consume(command); }`); + expect(cfg.refinementSafe).toBe(false); + expect(createFunctionRefinement(cfg).guards.status).toBe('fallback'); + }); + it('preserves guard and value identity through embedded coordinate mappings', () => { const root = parse(`const SHA_RE = /^[0-9a-f]{7,40}$/; function f(ref: string) { diff --git a/gitnexus/test/unit/taint/guard-refinement.test.ts b/gitnexus/test/unit/taint/guard-refinement.test.ts index e21f4e945..f9fe3e069 100644 --- a/gitnexus/test/unit/taint/guard-refinement.test.ts +++ b/gitnexus/test/unit/taint/guard-refinement.test.ts @@ -109,6 +109,20 @@ describe('accepted-edge guard proof', () => { expect(computeGuardRefinements(cfg, facts).at(subject, sink)).toBeUndefined(); }); + it('charges transitive alias expansion to the same traversal budget', () => { + const { cfg, facts, subject } = fixture(); + const aliases = new Map( + Array.from({ length: 200 }, (_, index) => [ + subject + index, + { kind: 'binding' as const, binding: subject + index + 1 }, + ]), + ); + const proof = computeGuardRefinements(cfg, facts, { maxVisits: 128 }, aliases); + expect(proof.status).toBe('fallback'); + expect(proof.visits).toBeLessThanOrEqual(129); + expect(proof.at(subject, cfg.entryIndex)).toBeUndefined(); + }); + it('does not certify reassigned parameters or unknown primitive identity', () => { const { cfg, facts, subject, sink } = fixture( 'if (!valid(x)) return; x = input(); consume(x);', diff --git a/gitnexus/test/unit/taint/propagate.test.ts b/gitnexus/test/unit/taint/propagate.test.ts index 3ab469240..6c90673db 100644 --- a/gitnexus/test/unit/taint/propagate.test.ts +++ b/gitnexus/test/unit/taint/propagate.test.ts @@ -546,6 +546,36 @@ describe('multi-source identity — distinct sources do not merge at one def', ( // ── kind-set exclusion model (real built-in model) ────────────────────────── describe('kind-set exclusions — sanitizers neutralize their kinds only', () => { + it.each([ + ['direct alias', 'const command = "git show " + ref;', 'command', 0], + [ + 'transitive alias', + 'const args = "show " + ref; const command = "git " + args;', + 'command', + 0, + ], + ['template alias', 'const command = `git show ${ref}`;', 'command', 0], + ['unsafe template', 'const command = `git show ;${ref}`;', 'command', 1], + ['bypassed guard', 'const command = "git show " + ref;', 'command', 1], + ['mutated subject', 'ref = req.query.other; const command = "git show " + ref;', 'command', 1], + ['mutated alias', 'let command = "git show " + ref; command = req.query.other;', 'command', 1], + ])('keeps certificate semantics across blocks: %s', (name, definition, argument, findings) => { + const guard = 'if (!/^[0-9a-f]{7,40}$/.test(ref)) return;'; + const r = analyze( + `import {execSync} from 'node:child_process'; +function f(req: any, log: boolean) { + ${name === 'mutated subject' ? 'let' : 'const'} ref: string = req.query.ref; + ${name === 'bypassed guard' ? `if (log) { ${guard} }` : guard} + ${definition} + if (log) console.log('validated'); + execSync(${argument}); +}`, + { spec: TS_JS_TAINT_MODEL }, + ); + expect(r.status).toBe('computed'); + expect(r.findings).toHaveLength(findings); + }); + it('built-in model catches argv-form child_process command sinks', () => { const r = analyze( `import { execFileSync } from 'node:child_process';