Skip to content

Commit 4f2ddd6

Browse files
Bowen-Xue-ac2Bowen-Xue-ac2
committed
fix(telemetry): address credential masking review
1 parent c1e089e commit 4f2ddd6

2 files changed

Lines changed: 103 additions & 30 deletions

File tree

‎packages/opencode/src/altimate/telemetry/error-detail.ts‎

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,38 @@
11
import { Telemetry } from "./index"
2+
import stripAnsi from "strip-ansi"
23

34
const MAX_MESSAGE = 200
45
const PLACEHOLDER = "<x>"
5-
const CREDENTIAL_WORDS: ReadonlySet<string> = new Set([
6+
export const CREDENTIAL_WORDS: ReadonlySet<string> = new Set([
7+
"accesstoken",
8+
"apikey",
9+
"appsecret",
610
"auth",
711
"authorization",
12+
"authtoken",
813
"bearer",
14+
"bearertoken",
15+
"clientsecret",
16+
"connectionstring",
917
"cookie",
1018
"credential",
1119
"credentials",
20+
"jwtsecret",
21+
"jwttoken",
1222
"key",
23+
"pass",
1324
"passphrase",
1425
"passwd",
1526
"password",
27+
"passwords",
1628
"pwd",
29+
"refreshtoken",
1730
"secret",
31+
"secretkey",
1832
"session",
33+
"sessionid",
34+
"sig",
35+
"signature",
1936
"token",
2037
])
2138

@@ -390,14 +407,21 @@ export const ERROR_VOCABULARY: ReadonlySet<string> = new Set([
390407
])
391408

392409
/**
393-
* Fixed-vocabulary template of an error's first line. Words are split on any non-letter
394-
* character and lowercased; a word survives only if it is in ERROR_VOCABULARY, every other
395-
* run becomes `<x>`. A credential word and the rest of the line become one `<x>` too,
396-
* because a credential value may itself consist of vocabulary words. The output holds
410+
* Fixed-vocabulary template of an error's first line, with ANSI escapes removed.
411+
* Words are split on any non-letter character and lowercased; a word survives only if
412+
* it is in ERROR_VOCABULARY, every other run becomes `<x>`. A credential word and the
413+
* rest of the line become one `<x>` too, because a credential value may itself consist
414+
* of vocabulary words. This deliberately loses diagnostic text after ambiguous words
415+
* such as key or session: privacy takes priority over template detail. The output holds
397416
* only vocabulary words, `<x>` and spaces.
398417
*/
399418
export function templateErrorMessage(raw: string): string {
400-
const first = raw.split(/[\r\n\u2028\u2029]/, 1)[0]
419+
// Split first so an OSC sequence spanning a newline cannot erase the line boundary.
420+
// Normalize only this compound credential key; prose "connection string" stays readable.
421+
const first = stripAnsi(raw.split(/[\r\n\u2028\u2029]/, 1)[0]).replace(
422+
/\bconnection[_-]string\b/gi,
423+
"connectionstring",
424+
)
401425
const out: string[] = []
402426
for (const token of first.split(/[^A-Za-z]+/)) {
403427
if (!token) continue

‎packages/opencode/test/telemetry/error-detail.test.ts‎

Lines changed: 73 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,8 @@
11
import { describe, expect, test } from "bun:test"
2-
import { Telemetry } from "../../src/altimate/telemetry"
32
import { ErrorDetail } from "../../src/altimate/telemetry/error-detail"
43

5-
const { buildErrorFingerprintEvent, describeToolError, templateErrorMessage, ERROR_VOCABULARY } = ErrorDetail
6-
7-
const CREDENTIAL_WORDS = [
8-
"password",
9-
"passwd",
10-
"pwd",
11-
"secret",
12-
"token",
13-
"key",
14-
"auth",
15-
"authorization",
16-
"credential",
17-
"credentials",
18-
"bearer",
19-
"cookie",
20-
"session",
21-
"passphrase",
22-
]
4+
const { buildErrorFingerprintEvent, describeToolError, templateErrorMessage, ERROR_VOCABULARY, CREDENTIAL_WORDS } =
5+
ErrorDetail
236

247
describe("templateErrorMessage", () => {
258
test.each([
@@ -31,7 +14,7 @@ describe("templateErrorMessage", () => {
3114
expect(templateErrorMessage(raw)).toBe(expected)
3215
})
3316

34-
test.each(CREDENTIAL_WORDS)("masks vocabulary words after %s", (word) => {
17+
test.each([...CREDENTIAL_WORDS])("masks vocabulary words after %s", (word) => {
3518
expect(templateErrorMessage(`request failed with ${word}: connection refused`)).toBe("request failed with <x>")
3619
})
3720

@@ -55,6 +38,70 @@ describe("templateErrorMessage", () => {
5538
)
5639
})
5740

41+
test.each([
42+
"apikey",
43+
"apiKey",
44+
"accessToken",
45+
"authToken",
46+
"refreshToken",
47+
"bearerToken",
48+
"jwtToken",
49+
"jwtSecret",
50+
"clientSecret",
51+
"appSecret",
52+
"signature",
53+
"sig",
54+
"pass",
55+
"passwords",
56+
"secretKey",
57+
"sessionId",
58+
"connectionString",
59+
])("masks the exact credential alias %s", (key) => {
60+
expect(templateErrorMessage(`request failed: ${key}=wrong user`)).toBe("request failed <x>")
61+
expect(templateErrorMessage(`${key}: connection refused`)).toBe("<x>")
62+
})
63+
64+
test.each([
65+
"request failed: connection_string=wrong user",
66+
"request failed: CONNECTION_STRING: connection refused",
67+
'request failed: "connection_string": "wrong user"',
68+
"request failed: connection-string=wrong user",
69+
"request failed: connection_\x1b[31mstring=wrong user",
70+
])("masks the exact connection string key: %j", (raw) => {
71+
expect(templateErrorMessage(raw)).toBe("request failed <x>")
72+
})
73+
74+
test.each([
75+
["connection string is invalid", "connection string is invalid"],
76+
["connection refused", "connection refused"],
77+
["signal received", "signal received"],
78+
["connection_stringify is invalid", "connection <x> is invalid"],
79+
["disconnection_string is invalid", "<x> string is invalid"],
80+
["connection_strings are invalid", "connection <x> are invalid"],
81+
["apikeyboard sigil signatured tokenizer failed", "<x> failed"],
82+
])("keeps diagnostics without an exact credential marker: %s", (raw, expected) => {
83+
expect(templateErrorMessage(raw)).toBe(expected)
84+
})
85+
86+
test.each([
87+
"request failed: \x1b[31mpassword\x1b[0m=wrong user",
88+
"request failed: pass\x1b[31mword=wrong user",
89+
"request failed: api\x1b[31mKey=wrong user",
90+
"request failed: \x1b[1;31mpassword\x1b[0m=connection refused",
91+
"request failed: \x1b]8;;https://example.test\x07password\x1b]8;;\x07=wrong user",
92+
"request failed: \x9b31mpassword\x9b0m=wrong user",
93+
])("ignores ANSI sequences when matching credentials: %j", (raw) => {
94+
expect(templateErrorMessage(raw)).toBe("request failed <x>")
95+
})
96+
97+
test("keeps diagnostic words inside ANSI styling", () => {
98+
expect(templateErrorMessage("connection \x1b[31mrefused\x1b[0m")).toBe("connection refused")
99+
})
100+
101+
test.each(["\n", "\r", "\r\n", "\u2028", "\u2029"])("does not let an OSC sequence consume %j", (newline) => {
102+
expect(templateErrorMessage(`first line \x1b]8;;${newline}\x07wrong user`)).toBe("first line")
103+
})
104+
58105
const adversarial = [
59106
"password: hunter",
60107
"password: supersecret",
@@ -86,7 +133,7 @@ describe("templateErrorMessage", () => {
86133
expect(tok === "<x>" || ERROR_VOCABULARY.has(tok)).toBe(true)
87134
}
88135
expect(out).not.toContain("<x> <x>")
89-
for (const w of CREDENTIAL_WORDS) expect(out).not.toContain(w)
136+
for (const w of CREDENTIAL_WORDS) expect(out.split(" ")).not.toContain(w)
90137
})
91138

92139
test("user words never survive", () => {
@@ -137,8 +184,10 @@ describe("error_fingerprint wiring", () => {
137184
expect("stack" in ev).toBe(false)
138185
})
139186

140-
test("credential suffix is hidden in the event without changing the hash", () => {
141-
const raw = "request failed: password is connection refused"
187+
test.each([
188+
"request failed: password is connection refused",
189+
"request failed: \x1b[31mpassword\x1b[0m is connection refused",
190+
])("credential suffix is hidden in the event without changing the hash: %j", (raw) => {
142191
const detail = describeToolError(raw)
143192
const event = buildErrorFingerprintEvent("s1", {
144193
...detail,
@@ -150,7 +199,7 @@ describe("error_fingerprint wiring", () => {
150199
})
151200
expect(event).toMatchObject({
152201
error_message: "request failed <x>",
153-
error_hash: Telemetry.hashError(Telemetry.maskString(raw).slice(0, 500)),
202+
error_hash: "ae7e754cfb2d72a2",
154203
})
155204
})
156205

0 commit comments

Comments
 (0)