Skip to content

Commit 925b648

Browse files
committed
do not redact feature flag style env vars
1 parent a7535e9 commit 925b648

9 files changed

Lines changed: 206 additions & 32 deletions

apps/sim/executor/handlers/agent/memory.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -337,8 +337,8 @@ describe('Memory', () => {
337337
expect(result.content).toBe('foreign-secret')
338338
})
339339

340-
it.each(['123', 'true'])(
341-
'projects low-entropy secret %s only in model text and arguments',
340+
it.each(['123'])(
341+
'projects short secret %s only in model text and arguments',
342342
async (secret) => {
343343
const registry = new ResolvedSecretTraceRegistry([
344344
{ name: 'TOKEN', plaintext: secret, encryptedValue: 'ciphertext' },

apps/sim/executor/utils/resolved-secret-content-projection.test.ts

Lines changed: 70 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,9 @@
33
*/
44
import { describe, expect, it, vi } from 'vitest'
55
import {
6+
createResolvedSecretMatcher,
67
isResolvedSecretModelContentUnchanged,
8+
projectResolvedSecretContent,
79
projectResolvedSecretDiagnosticError,
810
projectResolvedSecretModelContent,
911
projectResolvedSecretModelJsonContent,
@@ -175,7 +177,7 @@ describe('projectResolvedSecretModelContent', () => {
175177
})
176178
})
177179

178-
it('projects exact typed primitive secrets without rewriting unrelated primitives', () => {
180+
it('projects exact typed numeric secrets, leaving booleans and null identifying nothing', () => {
179181
const registry = new ResolvedSecretTraceRegistry([
180182
{ name: 'NUMBER', plaintext: '123', encryptedValue: 'number-ciphertext' },
181183
{ name: 'BOOLEAN', plaintext: 'true', encryptedValue: 'boolean-ciphertext' },
@@ -200,17 +202,17 @@ describe('projectResolvedSecretModelContent', () => {
200202
).toEqual({
201203
safe: true,
202204
value: {
203-
strings: ['{{NUMBER}}', '{{BOOLEAN}}', '{{NULL}}'],
205+
strings: ['{{NUMBER}}', 'true', 'null'],
204206
number: '{{NUMBER}}',
205-
boolean: '{{BOOLEAN}}',
206-
nothing: '{{NULL}}',
207+
boolean: true,
208+
nothing: null,
207209
unrelatedNumber: 1234,
208210
unrelatedBoolean: false,
209211
},
210212
})
211213
})
212214

213-
it.each(['123', 'true'])('keeps projected JSON argument strings valid (%s)', (secret) => {
215+
it.each(['123'])('keeps projected JSON argument strings valid (%s)', (secret) => {
214216
const registry = new ResolvedSecretTraceRegistry([
215217
{ name: 'TOKEN', plaintext: secret, encryptedValue: 'ciphertext' },
216218
])
@@ -231,6 +233,26 @@ describe('projectResolvedSecretModelContent', () => {
231233
})
232234
})
233235

236+
it('leaves a boolean-valued secret in a JSON argument string untouched', () => {
237+
const registry = new ResolvedSecretTraceRegistry([
238+
{ name: 'TOKEN', plaintext: 'true', encryptedValue: 'ciphertext' },
239+
])
240+
registry.recordResolved('TOKEN', 'true')
241+
242+
const projection = projectResolvedSecretModelJsonStrings(
243+
[JSON.stringify({ secret: 'true', converted: true, nested: [true] })],
244+
registry
245+
)
246+
247+
expect(projection.safe).toBe(true)
248+
if (!projection.safe || !Array.isArray(projection.value)) return
249+
expect(JSON.parse(projection.value[0] as string)).toEqual({
250+
secret: 'true',
251+
converted: true,
252+
nested: [true],
253+
})
254+
})
255+
234256
it('is stable when a secret literal overlaps its own provenance alias', () => {
235257
const registry = new ResolvedSecretTraceRegistry([
236258
{ name: 'TOKEN', plaintext: 'TOKEN', encryptedValue: 'ciphertext' },
@@ -464,3 +486,46 @@ describe('projectResolvedSecretDiagnosticError', () => {
464486
})
465487
})
466488
})
489+
490+
describe('literals too small to identify anything', () => {
491+
const matcher = createResolvedSecretMatcher(
492+
[
493+
{ plaintext: 'false', replacement: '{{BANNER_ENABLED}}' },
494+
{ plaintext: 'xoxb-real-secret-value', replacement: '{{SLACK_TOKEN}}' },
495+
],
496+
{ preserveNamedProvenanceLabels: true, mode: 'render' }
497+
)!
498+
499+
const project = (value: unknown) =>
500+
projectResolvedSecretContent(value, matcher, 1_000_000, { projectPrimitiveLiterals: true })
501+
502+
/** A `*_ENABLED` variable holding `false` once rewrote 2,000 boolean cells in one table read. */
503+
it('leaves a typed boolean cell alone', () => {
504+
expect(project({ had_error: false, ok: true, missing: null })).toEqual({
505+
safe: true,
506+
value: { had_error: false, ok: true, missing: null },
507+
})
508+
})
509+
510+
it('leaves a delimited occurrence inside surrounding text alone', () => {
511+
expect(project({ url: 'https://x?fromUser=false&sort=count' })).toEqual({
512+
safe: true,
513+
value: { url: 'https://x?fromUser=false&sort=count' },
514+
})
515+
})
516+
517+
it('still substitutes a real secret sharing the same matcher', () => {
518+
expect(project({ token: 'xoxb-real-secret-value', flag: false })).toEqual({
519+
safe: true,
520+
value: { token: '{{SLACK_TOKEN}}', flag: false },
521+
})
522+
})
523+
524+
it('builds no matcher at all when every literal is non-identifying', () => {
525+
expect(
526+
createResolvedSecretMatcher([{ plaintext: 'true', replacement: '{{FLAG}}' }], {
527+
mode: 'render',
528+
})
529+
).toBeUndefined()
530+
})
531+
})

apps/sim/executor/utils/resolved-secret-match-policy.test.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import { describe, expect, it } from 'vitest'
55
import {
66
getResolvedSecretMatchPolicy,
7+
isNonIdentifyingSecretLiteral,
78
isWordBoundaryMatch,
89
MIN_UNANCHORED_MATCH_LENGTH,
910
} from '@/executor/utils/resolved-secret-match-policy'
@@ -89,3 +90,19 @@ describe('isWordBoundaryMatch', () => {
8990
expect(isWordBoundaryMatch('abc', 3, 3)).toBe(true)
9091
})
9192
})
93+
94+
describe('isNonIdentifyingSecretLiteral', () => {
95+
it.each(['true', 'false', 'null'])(
96+
'excludes %s, whose value space is too small to identify',
97+
(literal) => {
98+
expect(isNonIdentifyingSecretLiteral(literal)).toBe(true)
99+
}
100+
)
101+
102+
it.each(['0', '1', 'False', 'TRUE', 'Null', 'nullish', '', 'hunter2', 'sk_live_abc'])(
103+
'keeps %s protectable',
104+
(literal) => {
105+
expect(isNonIdentifyingSecretLiteral(literal)).toBe(false)
106+
}
107+
)
108+
})

apps/sim/executor/utils/resolved-secret-match-policy.ts

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,37 @@ export type ResolvedSecretMatchPolicy = 'anywhere' | 'boundary'
3535
*/
3636
export const MIN_UNANCHORED_MATCH_LENGTH = 8
3737

38+
/**
39+
* Literals that may not match at all, at any offset, because they identify nothing.
40+
*
41+
* This is cardinality, not entropy — the distinction the floor above turns on. An all-`f` HMAC key
42+
* is low-entropy but drawn from an enormous space, so a hit on it is evidence. `false` is drawn
43+
* from a space of two: a hit on it is evidence of nothing, and substituting it protects nothing an
44+
* attacker could not guess by flipping a coin. Meanwhile it rewrites every boolean any workflow
45+
* ever wrote — one deployment turned 2,000 `had_error` cells into `[REDACTED_SECRET]` because a
46+
* `*_BANNER_ENABLED` variable happened to hold `false`.
47+
*
48+
* Exactly the three JSON renderings of a non-string primitive, and nothing else. `0` and `1` are
49+
* deliberately absent: a short numeric secret is entirely plausible where a boolean one is not.
50+
* Matching is case-sensitive because the set is defined by what `String(value)` produces for a
51+
* typed primitive, not by what looks boolean — an environment variable literally holding `False`
52+
* keeps its protection.
53+
*
54+
* The residual is one bit: a variable whose whole value is the string `false` is no longer hidden.
55+
*/
56+
const NON_IDENTIFYING_SECRET_LITERALS: ReadonlySet<string> = new Set(['true', 'false', 'null'])
57+
58+
/**
59+
* True when a literal carries too little information to be worth protecting anywhere.
60+
*
61+
* Applied where literals are turned into matchers, so it governs detection and substitution alike:
62+
* such a value is never rewritten out of content, and never recorded into durable provenance as
63+
* something a later read must redact.
64+
*/
65+
export function isNonIdentifyingSecretLiteral(plaintext: string): boolean {
66+
return NON_IDENTIFYING_SECRET_LITERALS.has(plaintext)
67+
}
68+
3869
/**
3970
* Combining marks count so a substitution cannot split a grapheme cluster. `_` deliberately does
4071
* NOT: `sk_live_...` and `user_483920_profile` are the dominant way a secret gets joined into an

apps/sim/executor/utils/resolved-secret-matcher.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { MAX_INLINE_MATERIALIZATION_BYTES } from '@/lib/execution/payloads/limits'
22
import {
33
getResolvedSecretMatchPolicy,
4+
isNonIdentifyingSecretLiteral,
45
type ResolvedSecretMatchPolicy,
56
satisfiesResolvedSecretMatchPolicy,
67
} from '@/executor/utils/resolved-secret-match-policy'
@@ -457,7 +458,12 @@ export function createResolvedSecretMatcher(
457458
const replacementByPlaintext = new Map<string, string>()
458459

459460
for (const match of matches) {
460-
if (!match.plaintext) continue
461+
/**
462+
* Dropped before any construction-time check runs, so no later stage can be talked into
463+
* treating one of these as protectable — including the wide-match-set checks below, which
464+
* deliberately ignore the narrow policy.
465+
*/
466+
if (!match.plaintext || isNonIdentifyingSecretLiteral(match.plaintext)) continue
461467
const current = replacementByPlaintext.get(match.plaintext)
462468
if (current === undefined || compareStrings(match.replacement, current) < 0) {
463469
replacementByPlaintext.set(match.plaintext, match.replacement)

apps/sim/executor/utils/resolved-secret-trace-registry.test.ts

Lines changed: 58 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1010,11 +1010,11 @@ describe('ResolvedSecretTraceRegistry', () => {
10101010

10111011
it('conservatively retains every active secret that shares a raw plaintext literal', () => {
10121012
const registry = new ResolvedSecretTraceRegistry([
1013-
{ name: 'FIRST', plaintext: 'true', encryptedValue: 'first-ciphertext' },
1014-
{ name: 'SECOND', plaintext: 'true', encryptedValue: 'second-ciphertext' },
1013+
{ name: 'FIRST', plaintext: '4815162342', encryptedValue: 'first-ciphertext' },
1014+
{ name: 'SECOND', plaintext: '4815162342', encryptedValue: 'second-ciphertext' },
10151015
])
1016-
registry.recordResolved('FIRST', 'true')
1017-
registry.recordResolved('SECOND', 'true')
1016+
registry.recordResolved('FIRST', '4815162342')
1017+
registry.recordResolved('SECOND', '4815162342')
10181018

10191019
const expected = {
10201020
version: 1 as const,
@@ -1024,11 +1024,11 @@ describe('ResolvedSecretTraceRegistry', () => {
10241024
{ name: 'SECOND', encryptedValue: 'second-ciphertext' },
10251025
],
10261026
}
1027-
expect(registry.exportCommittedProvenanceForValue('true')).toEqual(expected)
1028-
expect(registry.exportCommittedProvenanceForValue(true)).toEqual(expected)
1027+
expect(registry.exportCommittedProvenanceForValue('4815162342')).toEqual(expected)
1028+
expect(registry.exportCommittedProvenanceForValue(4815162342)).toEqual(expected)
10291029
})
10301030

1031-
it('exports active numeric, boolean, and null literals crossing a value boundary', () => {
1031+
it('exports active numeric literals crossing a value boundary, but not boolean or null', () => {
10321032
const registry = new ResolvedSecretTraceRegistry([
10331033
{ name: 'NUMBER', plaintext: '1234', encryptedValue: 'number-ciphertext' },
10341034
{ name: 'BOOLEAN', plaintext: 'false', encryptedValue: 'boolean-ciphertext' },
@@ -1048,11 +1048,7 @@ describe('ResolvedSecretTraceRegistry', () => {
10481048
).toEqual({
10491049
version: 1,
10501050
complete: true,
1051-
entries: [
1052-
{ encryptedValue: 'boolean-ciphertext' },
1053-
{ encryptedValue: 'null-ciphertext' },
1054-
{ encryptedValue: 'number-ciphertext' },
1055-
],
1051+
entries: [{ encryptedValue: 'number-ciphertext' }],
10561052
})
10571053
})
10581054

@@ -1576,3 +1572,53 @@ describe('incompleteness diagnostics', () => {
15761572
expect(logged).not.toContain('MISSING')
15771573
})
15781574
})
1575+
1576+
describe('non-identifying literals in durable provenance', () => {
1577+
/**
1578+
* The amplifier behind the boolean redaction: once recorded on a row, every later read of that
1579+
* table reactivated the value and rewrote every boolean in it.
1580+
*/
1581+
it('never records a value too small to identify anything', () => {
1582+
const registry = new ResolvedSecretTraceRegistry([
1583+
{ name: 'BANNER_ENABLED', plaintext: 'false', encryptedValue: 'flag-ciphertext' },
1584+
{ name: 'TOKEN', plaintext: 'xoxb-real-secret-value', encryptedValue: 'token-ciphertext' },
1585+
])
1586+
registry.recordResolved('BANNER_ENABLED', 'false')
1587+
registry.recordResolved('TOKEN', 'xoxb-real-secret-value')
1588+
1589+
expect(registry.exportProvenanceForValue({ had_error: false, note: 'fromUser=false' })).toEqual(
1590+
{ version: 1, complete: true, entries: [] }
1591+
)
1592+
expect(registry.exportProvenanceForValue({ token: 'xoxb-real-secret-value' })).toEqual({
1593+
version: 1,
1594+
complete: true,
1595+
entries: [{ name: 'TOKEN', encryptedValue: 'token-ciphertext' }],
1596+
})
1597+
})
1598+
1599+
it('still recognizes the internal alias, which names the variable its value cannot', () => {
1600+
const registry = new ResolvedSecretTraceRegistry([
1601+
{ name: 'BANNER_ENABLED', plaintext: 'false', encryptedValue: 'flag-ciphertext' },
1602+
])
1603+
registry.recordResolved('BANNER_ENABLED', 'false')
1604+
1605+
expect(registry.exportProvenanceForValue({ code: '__var_BANNER_ENABLED' })).toEqual({
1606+
version: 1,
1607+
complete: true,
1608+
entries: [{ name: 'BANNER_ENABLED', encryptedValue: 'flag-ciphertext' }],
1609+
})
1610+
})
1611+
1612+
it('keeps it out of the model matcher so nothing downstream can substitute it', () => {
1613+
const registry = new ResolvedSecretTraceRegistry([
1614+
{ name: 'BANNER_ENABLED', plaintext: 'false', encryptedValue: 'flag-ciphertext' },
1615+
])
1616+
registry.recordResolved('BANNER_ENABLED', 'false')
1617+
1618+
const snapshot = registry.getModelEgressSnapshot()
1619+
expect(snapshot.complete).toBe(true)
1620+
if (snapshot.complete) {
1621+
expect(snapshot.matches.map((match) => match.plaintext)).not.toContain('false')
1622+
}
1623+
})
1624+
})

apps/sim/executor/utils/resolved-secret-trace-registry.ts

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { decryptSecret } from '@/lib/core/security/encryption'
44
import { isLargeArrayManifest } from '@/lib/execution/payloads/large-array-manifest-metadata'
55
import { isLargeValueRef } from '@/lib/execution/payloads/large-value-ref'
66
import { MAX_INLINE_MATERIALIZATION_BYTES } from '@/lib/execution/payloads/limits'
7+
import { isNonIdentifyingSecretLiteral } from '@/executor/utils/resolved-secret-match-policy'
78
import {
89
createResolvedSecretMatcher,
910
OPAQUE_RESOLVED_SECRET_REPLACEMENT,
@@ -1303,7 +1304,12 @@ export class ResolvedSecretTraceRegistry {
13031304
private buildMatches(entries: Iterable<ActiveSecretEntry>): readonly ResolvedSecretTraceMatch[] {
13041305
const candidatesByPlaintext = new Map<string, ActiveSecretEntry[]>()
13051306
for (const entry of entries) {
1306-
if (entry.plaintext.length === 0) continue
1307+
/**
1308+
* Dropped here too, not only inside the matcher, so a literal that will never be substituted
1309+
* also never counts toward the matcher capacity bound or appears to a snapshot reader as
1310+
* something this registry protects.
1311+
*/
1312+
if (entry.plaintext.length === 0 || isNonIdentifyingSecretLiteral(entry.plaintext)) continue
13071313
const candidates = candidatesByPlaintext.get(entry.plaintext) ?? []
13081314
candidates.push(entry)
13091315
candidatesByPlaintext.set(entry.plaintext, candidates)
@@ -1511,7 +1517,12 @@ export class ResolvedSecretTraceRegistry {
15111517
compareStrings(left.encryptedValue, right.encryptedValue)
15121518
)
15131519
for (const entry of sortedCandidateEntries) {
1514-
if (entry.plaintext.length === 0) continue
1520+
/**
1521+
* Excluded from scan literals as well as from the matcher, so such a value is never recorded
1522+
* into durable provenance as something a later read must redact. A named entry still joins
1523+
* the alias loop below — `__var_NAME` identifies the variable even when its value does not.
1524+
*/
1525+
if (entry.plaintext.length === 0 || isNonIdentifyingSecretLiteral(entry.plaintext)) continue
15151526
const candidates = candidatesByPlaintext.get(entry.plaintext) ?? []
15161527
const entryKey = activeEntryKey(entry)
15171528
if (!candidates.some((candidate) => activeEntryKey(candidate) === entryKey)) {

apps/sim/lib/copilot/request/tools/resolved-secret-result.test.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -227,7 +227,7 @@ describe('projectToolResultForCopilot', () => {
227227
).toEqual({ success: true, output: { result: encoded } })
228228
})
229229

230-
it('projects exact typed primitive secrets and the same values as strings', () => {
230+
it('projects exact typed numeric secrets, leaving booleans and null identifying nothing', () => {
231231
const registry = new ResolvedSecretTraceRegistry([
232232
{ name: 'NUMBER', plaintext: '123', encryptedValue: 'number-ciphertext' },
233233
{ name: 'BOOLEAN', plaintext: 'true', encryptedValue: 'boolean-ciphertext' },
@@ -256,11 +256,11 @@ describe('projectToolResultForCopilot', () => {
256256
success: true,
257257
output: {
258258
number: '{{NUMBER}}',
259-
boolean: '{{BOOLEAN}}',
260-
nothing: '{{NULL}}',
259+
boolean: true,
260+
nothing: null,
261261
numberText: '{{NUMBER}}',
262-
booleanText: '{{BOOLEAN}}',
263-
nilText: '{{NULL}}',
262+
booleanText: 'true',
263+
nilText: 'null',
264264
},
265265
})
266266
})

0 commit comments

Comments
 (0)