feat(compression): risk-gate pre-pass — shield sensitive spans from lossy compression - #5243
Conversation
…markers, enabled guard)
There was a problem hiding this comment.
Code Review
This pull request introduces a 'risk-gate' pre-pass feature designed to shield sensitive data (such as private keys, secrets, stack traces, and database migrations) from lossy compression, restoring them verbatim afterward. It includes pattern matching, masking/restoring utilities, integration with the compression pipeline, and UI updates in the compression studio. The review feedback highlights several important improvements: resolving redundant double-wrapping of withRiskGate during stacked compression, moving PreviewRequestSchema to the shared validation directory to comply with Repository Rule 8, adding defensive guards against potential runtime TypeErrors when parsing messages, refining DDL span promotion to avoid shielding excessive intermediate text, and fixing the secret_assignment regex to properly match closing quotes.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| ): CompressionResult { | ||
| return withRiskGate(body, resolveRiskGate(options), (b) => runCompression(b, mode, options)); | ||
| } |
There was a problem hiding this comment.
When applyCompression is called with mode === "stacked", it wraps the execution with withRiskGate. However, runCompression then delegates to applyStackedCompression, which also wraps itself with withRiskGate. This results in redundant double-wrapping and executes the risk-gate regex matching twice on the same content, impacting performance. We should strip the riskGate option from the options passed to runCompression to prevent this double-wrapping.
): CompressionResult {
const gate = resolveRiskGate(options);
if (!gate) return runCompression(body, mode, options);
const { riskGate, ...rest } = options ?? {};
const cleanedConfig = options?.config ? { ...options.config, riskGate: undefined } : undefined;
const cleanedOptions = { ...rest, ...(cleanedConfig ? { config: cleanedConfig } : {}) };
return withRiskGate(body, gate, (b) => runCompression(b, mode, cleanedOptions));
}| // Playground risk-gate toggle → masks high-risk spans (secrets/keys) before compression and | ||
| // restores them verbatim after, so they pass through byte-identical. Reported via | ||
| // result.stats.riskGate (spansProtected + per-category counts). | ||
| riskGate: z.object({ enabled: z.boolean() }).optional(), |
There was a problem hiding this comment.
Defining PreviewRequestSchema locally in the route file violates Repository Rule 8, which states: 'Always validate inputs with Zod schemas from src/shared/validation/schemas.ts'. To adhere to the repository style guide, this schema should be moved to the shared validation schemas directory and imported here.
References
- Always validate inputs with Zod schemas from
src/shared/validation/schemas.ts.
| const maskedMessages = messages.map((msg) => { | ||
| const m = msg as { role?: unknown; content?: unknown }; | ||
| if (typeof m.content === "string") { |
There was a problem hiding this comment.
If msg is null or not an object at runtime, accessing m.content will throw a TypeError. We should add a defensive guard to ensure msg is a valid object before accessing its properties.
| const maskedMessages = messages.map((msg) => { | |
| const m = msg as { role?: unknown; content?: unknown }; | |
| if (typeof m.content === "string") { | |
| const maskedMessages = messages.map((msg) => { | |
| if (!msg || typeof msg !== "object") return msg; | |
| const m = msg as { role?: unknown; content?: unknown }; | |
| if (typeof m.content === "string") { |
| messages: messages.map((msg) => { | ||
| const m = msg as { content?: unknown }; | ||
| if (typeof m.content === "string" || Array.isArray(m.content)) { |
There was a problem hiding this comment.
If msg is null or not an object at runtime, accessing m.content will throw a TypeError. We should add a defensive guard to ensure msg is a valid object before accessing its properties.
| messages: messages.map((msg) => { | |
| const m = msg as { content?: unknown }; | |
| if (typeof m.content === "string" || Array.isArray(m.content)) { | |
| messages: messages.map((msg) => { | |
| if (!msg || typeof msg !== "object") return msg; | |
| const m = msg as { content?: unknown }; | |
| if (typeof m.content === "string" || Array.isArray(m.content)) { |
| const ddlPromoted: RiskSpan[] = | ||
| ddl.length >= MIN_DDL ? [{ start: ddl[0].start, end: ddl[ddl.length - 1].end, category: "db_migration" }] : []; |
There was a problem hiding this comment.
Promoting a single giant span from the first DDL statement to the last DDL statement (ddl[0].start to ddl[ddl.length - 1].end) is extremely blunt. If a document contains two DDL statements separated by a large amount of normal prose, the entire intermediate text will be shielded from compression. Instead, we should promote each DDL hit individually as its own span. mergeSpans will automatically merge them if they actually overlap or are adjacent.
| const ddlPromoted: RiskSpan[] = | |
| ddl.length >= MIN_DDL ? [{ start: ddl[0].start, end: ddl[ddl.length - 1].end, category: "db_migration" }] : []; | |
| const ddlPromoted: RiskSpan[] = | |
| ddl.length >= MIN_DDL ? ddl.map((h) => ({ start: h.start, end: h.end, category: "db_migration" })) : []; |
| regex: | ||
| /\b(?:api[_-]?key|secret|token|password|passwd|bearer|authorization|client[_-]?secret)\b[ \t]{0,20}[:=][ \t]{0,20}["']?[A-Za-z0-9._\-+/]{8,200}/gi, |
There was a problem hiding this comment.
The secret_assignment regex matches an optional opening quote ["']? but does not match the corresponding closing quote. This leaves the closing quote exposed to lossy compression, which can lead to syntax errors or broken quotes in the restored text if the lossy compressor decides to strip or modify the unmatched trailing quote. We should use a capture group and backreference to match matching quotes cleanly.
| regex: | |
| /\b(?:api[_-]?key|secret|token|password|passwd|bearer|authorization|client[_-]?secret)\b[ \t]{0,20}[:=][ \t]{0,20}["']?[A-Za-z0-9._\-+/]{8,200}/gi, | |
| regex: | |
| /\b(?:api[_-]?key|secret|token|password|passwd|bearer|authorization|client[_-]?secret)\b[ \t]{0,20}[:=][ \t]{0,20}(["']?)[A-Za-z0-9._\-+/]{8,200}\1/gi, |
Babysit summary — risk-gate (#5)CI verdict: all checks green except the one expected, owner-handled red.
The single red is Verified the failing step is file-size and nothing else via Local full battery (all green): lint 0 · typecheck:core · check:cycles · check:complexity 1980/1980 · check:cognitive-complexity 841/841 · 19 node tests + 2 vitest UI · byte-identical parity when gate disabled · Ready for human review & merge. Not auto-merging. |
…e-pass wiring) The risk-gate mask->run->restore wrapper extracts the 3 entry points into thin wrappers over pure private bodies so the gate sits outside the per-step loop; the +45 residual is dispatch-boundary wiring guarded by the byte-identical parity test. Default off. Structural shrink tracked in #3501.
…ossy compression (diegosouzapw#5243) Risk-gate pre-pass — shields sensitive spans (PEM/secret/stack/k8s/migration/legal) from lossy compression via SENTINEL preserveSpans. Default off, fail-open, ReDoS-bounded patterns. strategySelector baseline rebaselined for the wrapper extraction.
What
Adds an opt-in, fail-open "risk-gate" pre-pass to the compression subsystem that surgically shields sensitive spans from lossy compression while letting the rest of the text compress normally. 7th feature of the compression feature-extraction roadmap (bench: #5080, fidelity gate: #5127, fuzzy: #5143, ionizer: #5148, TOON: #5163, CCR ranged: #5187).
Today the compressor has zero awareness of content risk — aggressive/
ultra/ionizer-sampling modes can mangle or drop a secret, a private key, a stack trace, a k8s Secret, a DB migration, or license text. The fidelity gate (#5127) only catches corruption after the fact and a lossy-by-design engine bypasses it. The risk-gate prevents the damage up front. Default off — flipping it on is per-operator (config/env), never silent.How
A new
riskGate/module (peer to the fidelity gate), wired as an outer mask→run→restore wrapper around the three compression entry points — a single, universal integration point:riskGate/riskPatterns.ts—RiskCategorycatalog + per-category bounded regexes (every variable-length pattern uses{0,N}quantifiers → no ReDoS) + the self-evident set.riskGate/riskGate.ts—detectRiskSpans(text, cfg): structural detection with a multi-signal guard (self-evident categoriesprivate_key/k8s_secret/db_migration(≥2 DDL)promote alone;secret_assignment/stack_trace/legalneed ≥2 corroborating signals or a short <200-char section) + a commit-log/diff guard (drops DDL that only appears inside adiff --githunk; strong VCS markers only). Pure, fail-open (never throws).riskGate/riskGateStep.ts—applyRiskMask(body, cfg)masks detected spans across message content (string and{type:"text"}multimodal parts) into SENTINEL placeholders,restoreRiskBlocksrestores them byte-identically.riskGate/strategyWrap.ts—resolveRiskGate+withRiskGate/withRiskGateAsync(kept out ofstrategySelectorto minimize its growth).preservation.ts— one new exported helperpreserveSpans(text, spans)that wraps literal offsets into the exactOMNI_CAVEMANSENTINEL family every engine already treats as opaque — so the secret survives caveman/ultra/etc. with zero per-engine changes (proven by a real-engine integration test).strategySelector.ts— the three exported entry points (applyCompression,applyStackedCompression,applyStackedCompressionAsync) become thin wrappers over pure-extracted private bodies (runCompression/runStackedCompression/runStackedCompressionAsync— identical logic, guarded by a byte-identical parity test). The gate sits strictly outside the per-step loop, so fidelity/gateAdvanceruns on the masked body unchanged.types.ts—CompressionConfig.riskGate?+CompressionStats.riskGate?(additive, optional;DEFAULT_COMPRESSION_CONFIGunchanged → default off)./api/compression/previewacceptsriskGate: { enabled }and returnsriskGatestats; the studio gets a toggle checkbox + a 🛡️ N risky spans protected badge.Security: the patterns are ours (not agent-supplied) and bounded; the gate only reduces risk surface (shields secrets from lossy transforms) and telemetry is counts + category names only (never the secret content). Error responses on the route stay routed through the untouched
sanitizeErrorMessagepath (Hard Rule #12).Tests (TDD, both runners)
tests/unit/compression/):riskGateDetect(11 — every category + the multi-signal/short-section/commit-log guards + bounded-regex adversarial input),riskGateStep(mask/restore byte-identical round-trip incl. multimodal + config defaults),riskGateIntegration(PEM stays byte-identical through real caveman while prose compresses; byte-identical to baseline when disabled; preview-route stats). 19/19 + 13 existing preservation/caveman suites stay green.tests/unit/ui/riskGateBadge.test.tsx): badge render/empty-state — 2/2.enabledshort-circuit, dead import) — all with tests intact.Gates (local)
lint 0 errors · typecheck:core clean · check:cycles OK · check:complexity 1980/1980 (baseline) · check:cognitive-complexity 841/841 (baseline) ·
next buildcompiles 596/596 static pages · no new dependency (nocheck:depschange).One expected red — file-size on
strategySelector.ts(own growth, please rebaseline at merge)open-sse/services/compression/strategySelector.ts: 899 > frozen 854. This is genuine own-growth from the wrapper wiring, already minimized (helpers extracted tostrategyWrap.ts; 929→898 lines). The residual is irreducible for the extract-to-runXpattern on a file already slated for structural decomposition (#3501). I did not editconfig/quality/file-size-baseline.json(baseline is yours) — please--admin/rebaseline at merge, same as #5143/#5148/#5163/#5187.Ready for review & merge.