Conversation
🦋 Changeset detectedLatest commit: e1159ab The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
There was a problem hiding this comment.
4 issues found across 12 files
Confidence score: 3/5
packages/cli/src/lib/secrets/api.ts/createSecretcan expose raw provider or network messages, including unwrapped JSON errors, when requests fail; normalize and sanitize these failures before surfacing them.packages/cli/src/lib/secrets/input.tscan treat inherited properties such asconstructorortoStringas environment values, leading to an opaqueERR_INVALID_ARG_TYPEinstead of the intended missing-variable error; validate that the value is an own string property.packages/cli/src/lib/secrets/seal.tsthrows a rawTypeErrorwhen the keypair endpoint returns a non-stringpublicKey, bypassing the intendedCommandFailure; type-check the value before callingBuffer.from.packages/cli/tests/secrets-input.test.tsuses an assertion that could still pass if cancellation handling appends the prompt error, leaving secret-text redaction regressions undetected; assert the exact sanitized error message.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/cli/src/lib/secrets/api.ts">
<violation number="1" location="packages/cli/src/lib/secrets/api.ts:66">
P1: Custom agent: **Exception and error message sanitization**
When either API request fails, `createSecret` lets `requestBrowserbaseJson` surface the cloud helper's raw provider/network message (and an unwrapped JSON parse error) directly to `BrowseCommand`, which prints it to the user. Wrap the secret-creation flow in a typed failure with a fixed sanitized message so errors cannot reflect secret-related request or provider details.</violation>
</file>
<file name="packages/cli/tests/secrets-input.test.ts">
<violation number="1" location="packages/cli/tests/secrets-input.test.ts:36">
P2: If cancellation handling starts appending the prompt error, this assertion still passes despite the test's redaction claim. Assert the exact error message so secret text regressions are caught.</violation>
</file>
<file name="packages/cli/src/lib/secrets/input.ts">
<violation number="1" location="packages/cli/src/lib/secrets/input.ts:11">
P2: When `--env` names an unset inherited property such as `constructor` or `toString`, this lookup returns a function and `Buffer.from` throws an opaque `ERR_INVALID_ARG_TYPE` instead of the intended missing-variable error. Check that the environment key is an own property before reading it.</violation>
</file>
<file name="packages/cli/src/lib/secrets/seal.ts">
<violation number="1" location="packages/cli/src/lib/secrets/seal.ts:9">
P2: When the keypair endpoint returns a non-string `publicKey`, `Buffer.from` throws before the invalid-key check and `try` block, exposing a raw `TypeError` instead of the intended `CommandFailure`. Guard the value's type before decoding it.</violation>
</file>
Architecture diagram
sequenceDiagram
participant User as CLI User
participant CLI as Browse CLI
participant Input as Secret Input Module
participant Seal as HPKE Seal Module
participant API as Browserbase API
Note over User,API: Create Secret Flow
User->>CLI: browse cloud secrets create SERVICE_TOKEN
CLI->>Input: readSecretValue({ stdin, env })
alt --env VARIABLE specified
Input->>Input: Read process.env[VARIABLE]
alt Variable not set
Input-->>CLI: Error: env variable not set
end
else --stdin flag set
Input->>Input: Read piped stdin bytes
alt stdin is TTY
Input-->>CLI: Error: requires piped input
end
else Interactive mode
alt stdin is not TTY
Input-->>CLI: Error: use --stdin or --env
else Prompt user
Input->>User: Hidden password prompt (stderr)
User-->>Input: Secret value
end
end
Input-->>CLI: Secret value (Uint8Array)
CLI->>API: GET /v1/secrets/keypair
API-->>CLI: { id: keypairId, publicKey }
alt Invalid public key format
CLI-->>User: Error: invalid X25519 public key
else Valid public key
CLI->>Seal: sealSecret(publicKey, value)
Seal->>Seal: Deserialize X25519 public key
Seal->>Seal: HPKE seal (DHKEM-X25519 + HKDF-SHA256 + AES-256-GCM)
Seal-->>CLI: sealedSecretValue (base64, enc + ciphertext)
CLI->>API: POST /v1/secrets (keypairId, secretKey, sealedSecretValue)
alt Success
API-->>CLI: Secret metadata
CLI-->>User: Output JSON
else Duplicate key
API-->>CLI: 409 Conflict
CLI-->>User: Error: Secret already exists
end
end
CLI->>CLI: Zero-fill secret value buffer
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| secretKey: string, | ||
| value: Uint8Array, | ||
| ): Promise<Secret> { | ||
| const keypair = await requestBrowserbaseJson<{ |
There was a problem hiding this comment.
P1: Custom agent: Exception and error message sanitization
When either API request fails, createSecret lets requestBrowserbaseJson surface the cloud helper's raw provider/network message (and an unwrapped JSON parse error) directly to BrowseCommand, which prints it to the user. Wrap the secret-creation flow in a typed failure with a fixed sanitized message so errors cannot reflect secret-related request or provider details.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/cli/src/lib/secrets/api.ts, line 66:
<comment>When either API request fails, `createSecret` lets `requestBrowserbaseJson` surface the cloud helper's raw provider/network message (and an unwrapped JSON parse error) directly to `BrowseCommand`, which prints it to the user. Wrap the secret-creation flow in a typed failure with a fixed sanitized message so errors cannot reflect secret-related request or provider details.</comment>
<file context>
@@ -56,3 +57,24 @@ export async function deleteSecret(
+ secretKey: string,
+ value: Uint8Array,
+): Promise<Secret> {
+ const keypair = await requestBrowserbaseJson<{
+ id: string;
+ publicKey: string;
</file context>
|
|
||
| it("reports cancellation without echoing the prompt error", async () => { | ||
| vi.mocked(password).mockRejectedValue(new Error("private-value")); | ||
| await expect(readSecretValue({})).rejects.toThrow( |
There was a problem hiding this comment.
P2: If cancellation handling starts appending the prompt error, this assertion still passes despite the test's redaction claim. Assert the exact error message so secret text regressions are caught.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/cli/tests/secrets-input.test.ts, line 36:
<comment>If cancellation handling starts appending the prompt error, this assertion still passes despite the test's redaction claim. Assert the exact error message so secret text regressions are caught.</comment>
<file context>
@@ -0,0 +1,50 @@
+
+ it("reports cancellation without echoing the prompt error", async () => {
+ vi.mocked(password).mockRejectedValue(new Error("private-value"));
+ await expect(readSecretValue({})).rejects.toThrow(
+ "Secret input cancelled.",
+ );
</file context>
| if (options.env !== undefined) { | ||
| if (options.stdin) fail("--env and --stdin cannot be used together."); | ||
| if (!options.env) fail("--env requires an environment variable name."); | ||
| const value = process.env[options.env]; |
There was a problem hiding this comment.
P2: When --env names an unset inherited property such as constructor or toString, this lookup returns a function and Buffer.from throws an opaque ERR_INVALID_ARG_TYPE instead of the intended missing-variable error. Check that the environment key is an own property before reading it.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/cli/src/lib/secrets/input.ts, line 11:
<comment>When `--env` names an unset inherited property such as `constructor` or `toString`, this lookup returns a function and `Buffer.from` throws an opaque `ERR_INVALID_ARG_TYPE` instead of the intended missing-variable error. Check that the environment key is an own property before reading it.</comment>
<file context>
@@ -0,0 +1,37 @@
+ if (options.env !== undefined) {
+ if (options.stdin) fail("--env and --stdin cannot be used together.");
+ if (!options.env) fail("--env requires an environment variable name.");
+ const value = process.env[options.env];
+ if (value === undefined)
+ fail("The environment variable selected by --env is not set.");
</file context>
| const value = process.env[options.env]; | |
| const value = Object.hasOwn(process.env, options.env) | |
| ? process.env[options.env] | |
| : undefined; |
| publicKey: string, | ||
| value: Uint8Array, | ||
| ): Promise<string> { | ||
| const rawKey = Buffer.from(publicKey, "base64"); |
There was a problem hiding this comment.
P2: When the keypair endpoint returns a non-string publicKey, Buffer.from throws before the invalid-key check and try block, exposing a raw TypeError instead of the intended CommandFailure. Guard the value's type before decoding it.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/cli/src/lib/secrets/seal.ts, line 9:
<comment>When the keypair endpoint returns a non-string `publicKey`, `Buffer.from` throws before the invalid-key check and `try` block, exposing a raw `TypeError` instead of the intended `CommandFailure`. Guard the value's type before decoding it.</comment>
<file context>
@@ -0,0 +1,32 @@
+ publicKey: string,
+ value: Uint8Array,
+): Promise<string> {
+ const rawKey = Buffer.from(publicKey, "base64");
+ if (rawKey.length !== 32 || rawKey.toString("base64") !== publicKey) {
+ fail("The secrets API returned an invalid X25519 public key.");
</file context>
why
We want to add secrets support to the browse cli and is a continuation of the efforts introduced in this PR:
#2946.
In this particular PR, we add support for creating a secret by retrieving the public key for the project, reading the secret value from an env variable, a value piped to stdin, or prompting them in a password prompt (the inquire package), encrypting the value with the public key, then calling the create secret endpoint with the secret key name and the encrypted value.
what changed
test plan