Skip to content

Commit d69f694

Browse files
committed
fix(vertex): drop path preview from warning + extract shared path-shape predicate
Three review items addressed: - vertex-credentials.ts: removed the dynamic preview of the user's input from the warning message — leaking usernames and directory names into extension logs is mild PII. The static message still identifies the correct field and the env var to use. - Extracted the path-shape predicate to @roo-code/types (packages/types/src/utils/looksLikeFilePath.ts) so the UI warning in Vertex.tsx and the runtime warning in parseVertexJsonCredentials share one implementation instead of drifting independently. Both call sites now import from the shared module; the predicate is pure, dep-free, regex-based, and covered by its own spec. - Vertex.spec.tsx: the <Trans> mock now resolves keys against the real webview-ui English settings.json so the test asserts the rendered warning text. A future translation regression that dropped the 'Google Cloud Key File Path' field name or the GOOGLE_APPLICATION_CREDENTIALS env-var mention from the English copy would now be caught.
1 parent 7912fb3 commit d69f694

7 files changed

Lines changed: 120 additions & 36 deletions

File tree

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
// npx vitest run src/__tests__/looksLikeFilePath.spec.ts
2+
3+
import { looksLikeFilePath } from "../utils/looksLikeFilePath.js"
4+
5+
describe("looksLikeFilePath", () => {
6+
describe("nullish, empty, and whitespace-only input", () => {
7+
it.each([
8+
["undefined", undefined],
9+
["null", null],
10+
["empty string", ""],
11+
["whitespace only", " \t\n "],
12+
])("returns false for %s", (_label, value) => {
13+
expect(looksLikeFilePath(value)).toBe(false)
14+
})
15+
})
16+
17+
describe("path-shaped input", () => {
18+
it.each([
19+
["Windows backslash path", "C:\\Users\\dev\\sa.json"],
20+
["Windows forward-slash path", "C:/Users/dev/sa.json"],
21+
["Windows drive lowercase", "d:\\creds.json"],
22+
["POSIX absolute path", "/home/dev/sa.json"],
23+
["POSIX absolute root /tmp", "/tmp/creds.json"],
24+
["POSIX home path", "~/sa.json"],
25+
["POSIX relative ./", "./sa.json"],
26+
["POSIX relative ../", "../secrets/sa.json"],
27+
])("returns true for %s", (_label, value) => {
28+
expect(looksLikeFilePath(value)).toBe(true)
29+
})
30+
31+
it("returns true after trimming surrounding whitespace", () => {
32+
expect(looksLikeFilePath(" /tmp/creds.json ")).toBe(true)
33+
expect(looksLikeFilePath("\tC:\\sa.json\n")).toBe(true)
34+
})
35+
})
36+
37+
describe("JSON-shaped or bare-token input", () => {
38+
it.each([
39+
["JSON object", '{"type":"service_account","client_email":"x@y.z"}'],
40+
["JSON array", "[1,2,3]"],
41+
["JSON with leading whitespace", ' {"type":"service_account"}'],
42+
["bare token", "not-json-and-not-a-path"],
43+
["service-account-style email", "sa@project.iam.gserviceaccount.com"],
44+
])("returns false for %s", (_label, value) => {
45+
expect(looksLikeFilePath(value)).toBe(false)
46+
})
47+
})
48+
})

packages/types/src/index.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,3 +33,5 @@ export * from "./vscode.js"
3333
export * from "./worktree.js"
3434

3535
export * from "./providers/index.js"
36+
37+
export * from "./utils/looksLikeFilePath.js"
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
// Returns true when a string is shaped like a filesystem path (Windows
2+
// drive-letter, POSIX absolute, POSIX home, POSIX relative). Pure and
3+
// dependency-free so it can be shared between the extension runtime (e.g.
4+
// parseVertexJsonCredentials) and the webview UI (e.g. the Vertex settings
5+
// warning), guaranteeing both surfaces agree on what "looks like a path"
6+
// means.
7+
//
8+
// Returns false for nullish, empty, and whitespace-only input — neither
9+
// call site should warn in those cases.
10+
export function looksLikeFilePath(value: string | null | undefined): boolean {
11+
if (value == null) {
12+
return false
13+
}
14+
const trimmed = value.trim()
15+
if (!trimmed) {
16+
return false
17+
}
18+
return (
19+
/^[A-Za-z]:[\\/]/.test(trimmed) || // Windows: C:\... or C:/...
20+
trimmed.startsWith("/") || // POSIX absolute: /home/...
21+
trimmed.startsWith("~") || // POSIX home: ~/...
22+
trimmed.startsWith(".") // POSIX relative: ./... or ../...
23+
)
24+
}

src/api/providers/__tests__/vertex-credentials.spec.ts

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -109,11 +109,18 @@ describe("parseVertexJsonCredentials", () => {
109109
expect(warnSpy).toHaveBeenCalledTimes(1)
110110
})
111111

112-
it("truncates long path previews in the warning", () => {
113-
const longPath = "/" + "a".repeat(200)
114-
parseVertexJsonCredentials(longPath)
112+
it("does not echo the user's path in the warning (no PII in extension logs)", () => {
113+
const sensitivePath = "/home/somerealuser/secrets/sa-key.json"
114+
parseVertexJsonCredentials(sensitivePath)
115+
expect(warnSpy).toHaveBeenCalledTimes(1)
115116
const [message] = warnSpy.mock.calls[0]
116-
expect(message).toContain("…")
117+
// The warning must identify the field and the env var, but must not
118+
// interpolate the user's actual input — usernames and directory names
119+
// would leak into extension logs otherwise.
120+
expect(message).toContain("Google Cloud Credentials")
121+
expect(message).toContain("GOOGLE_APPLICATION_CREDENTIALS")
122+
expect(message).not.toContain(sensitivePath)
123+
expect(message).not.toContain("somerealuser")
117124
})
118125

119126
it("falls back to the generic JSON parse error path for malformed but non-path input", () => {

src/api/providers/utils/vertex-credentials.ts

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import type { JWTInput } from "google-auth-library"
22

3+
import { looksLikeFilePath } from "@roo-code/types"
34
import { safeJsonParse } from "@roo-code/core"
45

56
// Detects when the "Google Cloud Credentials" field has received a filesystem
@@ -8,22 +9,22 @@ import { safeJsonParse } from "@roo-code/core"
89
// and the sibling "Google Cloud Key File Path" field is where a path belongs.
910
// Returns the parsed credentials object when the input looks like JSON, or
1011
// undefined when the field is empty, path-shaped, or unparseable.
12+
//
13+
// The path-shape predicate is shared with the webview UI warning via
14+
// @roo-code/types/looksLikeFilePath so both surfaces stay in agreement.
1115
export function parseVertexJsonCredentials(value: string | undefined): JWTInput | undefined {
1216
const trimmed = value?.trim()
1317
if (!trimmed) {
1418
return undefined
1519
}
1620

17-
const looksLikePath =
18-
/^[A-Za-z]:[\\/]/.test(trimmed) || // Windows: C:\... or C:/...
19-
trimmed.startsWith("/") || // POSIX absolute: /home/...
20-
trimmed.startsWith("~") || // POSIX home: ~/...
21-
trimmed.startsWith(".") // POSIX relative: ./... or ../...
22-
23-
if (looksLikePath) {
24-
const preview = trimmed.length > 40 ? `${trimmed.slice(0, 40)}…` : trimmed
21+
if (looksLikeFilePath(trimmed)) {
22+
// Intentionally static — the user's actual value is not interpolated
23+
// into the warning so usernames and directory names don't leak into
24+
// extension logs. The message still identifies the correct field and
25+
// the env var fallback.
2526
console.warn(
26-
`[Vertex] The 'Google Cloud Credentials' field appears to contain a file path ("${preview}"), ` +
27+
"[Vertex] The 'Google Cloud Credentials' field appears to contain a file path, " +
2728
"but this field expects the raw JSON contents of a service-account key file. " +
2829
"If you have a path to the credentials file, paste it into the 'Google Cloud Key File Path' field instead, " +
2930
"or leave both fields empty and use the GOOGLE_APPLICATION_CREDENTIALS environment variable.",

webview-ui/src/components/settings/providers/Vertex.tsx

Lines changed: 2 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -3,30 +3,13 @@ import { Trans } from "react-i18next"
33
import { Checkbox } from "vscrui"
44
import { VSCodeLink, VSCodeTextField } from "@vscode/webview-ui-toolkit/react"
55

6-
import { type ProviderSettings, VERTEX_REGIONS, VERTEX_1M_CONTEXT_MODEL_IDS } from "@roo-code/types"
6+
import { type ProviderSettings, VERTEX_REGIONS, VERTEX_1M_CONTEXT_MODEL_IDS, looksLikeFilePath } from "@roo-code/types"
77

88
import { useAppTranslation } from "@src/i18n/TranslationContext"
99
import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from "@src/components/ui"
1010

1111
import { inputEventTransform } from "../transforms"
1212

13-
// Detects when the "Google Cloud Credentials" field has received a filesystem
14-
// path instead of the raw JSON contents of a service-account key file. Mirrors
15-
// the runtime guard in src/api/providers/gemini.ts so the warning the user
16-
// sees in the UI matches what the runtime would log.
17-
function looksLikeFilePath(value: string): boolean {
18-
const trimmed = value.trim()
19-
if (!trimmed) {
20-
return false
21-
}
22-
return (
23-
/^[A-Za-z]:[\\/]/.test(trimmed) || // Windows: C:\... or C:/...
24-
trimmed.startsWith("/") || // POSIX absolute: /home/...
25-
trimmed.startsWith("~") || // POSIX home: ~/...
26-
trimmed.startsWith(".") // POSIX relative: ./... or ../...
27-
)
28-
}
29-
3013
type VertexProps = {
3114
apiConfiguration: ProviderSettings
3215
setApiConfigurationField: (field: keyof ProviderSettings, value: ProviderSettings[keyof ProviderSettings]) => void
@@ -54,7 +37,7 @@ export const Vertex = ({ apiConfiguration, setApiConfigurationField }: VertexPro
5437
)
5538

5639
const credentialsLooksLikePath = useMemo(
57-
() => looksLikeFilePath(apiConfiguration?.vertexJsonCredentials ?? ""),
40+
() => looksLikeFilePath(apiConfiguration?.vertexJsonCredentials),
5841
[apiConfiguration?.vertexJsonCredentials],
5942
)
6043

webview-ui/src/components/settings/providers/__tests__/Vertex.spec.tsx

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { render, screen } from "@testing-library/react"
22
import { Vertex } from "../Vertex"
33
import type { ProviderSettings } from "@roo-code/types"
44
import { VERTEX_REGIONS } from "@roo-code/types"
5+
import enSettings from "@src/i18n/locales/en/settings.json"
56

67
vi.mock("@vscode/webview-ui-toolkit/react", () => ({
78
VSCodeTextField: ({ children, value, onInput, type }: any) => (
@@ -27,10 +28,24 @@ vi.mock("@src/i18n/TranslationContext", () => ({
2728
}))
2829

2930
// The component uses <Trans> for the path-shape warning so it can interpolate
30-
// <strong>/<code> elements. The mock just renders the i18n key so the spec
31-
// can assert on its presence without depending on the English copy.
31+
// <strong>/<code> elements. Resolve the i18n key against the real English
32+
// resource so the test fails if the warning copy drops the "Google Cloud Key
33+
// File Path" field name or the "GOOGLE_APPLICATION_CREDENTIALS" env var
34+
// mention — those are the actual remediation hints users need.
3235
vi.mock("react-i18next", () => ({
33-
Trans: ({ i18nKey }: { i18nKey: string }) => <>{i18nKey}</>,
36+
Trans: ({ i18nKey }: { i18nKey: string }) => {
37+
// Keys are "<namespace>:<dotted.path>"; the spec only renders the
38+
// settings namespace so resolve against the imported English bundle.
39+
const [, dotted] = i18nKey.split(":")
40+
const resolved = dotted
41+
.split(".")
42+
.reduce<unknown>(
43+
(acc, segment) =>
44+
acc && typeof acc === "object" ? (acc as Record<string, unknown>)[segment] : undefined,
45+
enSettings,
46+
)
47+
return <>{typeof resolved === "string" ? resolved : i18nKey}</>
48+
},
3449
}))
3550

3651
vi.mock("@src/components/ui", () => ({
@@ -175,7 +190,11 @@ describe("Vertex", () => {
175190

176191
const warning = screen.getByTestId("vertex-credentials-path-warning")
177192
expect(warning).toBeInTheDocument()
178-
expect(warning.textContent).toContain("settings:providers.googleCloudCredentialsPathWarning")
193+
// The warning resolves through the real English bundle, so a
194+
// regression that dropped either of these remediation strings
195+
// from the translation would be caught here.
196+
expect(warning).toHaveTextContent(/Google Cloud Key File Path/)
197+
expect(warning).toHaveTextContent(/GOOGLE_APPLICATION_CREDENTIALS/)
179198
})
180199
})
181200
})

0 commit comments

Comments
 (0)