Skip to content

[SDK-7618] fix(security): send the CLI launch token on every gRPC call - #319

Merged
Dalwin-Barnard merged 3 commits into
mainfrom
security/SDK-7618-sdk-launch-token
Nov 6, 2026
Merged

Dalwin-Barnard merged 3 commits into
mainfrom
security/SDK-7618-sdk-launch-token

Conversation

@rounak610

@rounak610 rounak610 commented Oct 29, 2026 •

Copy link
Copy Markdown
Collaborator

What is this about?

Protects the service's local gRPC connection to the BrowserStack CLI binary with a per-launch token (SDK-7618, CWE-306). Paired with browserstack-binary#1984, which rejects calls without it.

  • BrowserstackCLI.start() generates a token (randomBytes(32), hex) and sets BROWSERSTACK_CLI_AUTH_TOKEN in process.env before spawning the binary, so the binary and every wdio worker inherit it.
  • GrpcClient.connect() passes one grpc-js interceptor to the SDKClient stub that adds x-browserstack-cli-token to every call.
  • There is no opt-out: every spawn gets a fresh token. Dev mode (BROWSERSTACK_CLI_ENV=development) generates nothing, because it connects to a separately started binary.

Older binaries ignore the extra header, so this can release before or after the binary. Sibling SDK PRs: #326 (v8 port), https://github.com/browserstack/browserstack-python-sdk/pull/1353, https://github.com/browserstack/browserstack-node-agent/pull/2780, https://github.com/browserstack/browserstack-ruby-sdk/pull/221, https://github.com/browserstack/browserstack-javaagent/pull/2363, https://github.com/browserstack/browserstack-csharp-sdk/pull/1429

Related Jira task/s

Release (mandatory for every PR — required for the ready-for-review label)

Version bump: (required — tick exactly one)

  • minor (backwards-compatible feature)
  • patch (bug fix or other small change)

Release notes type: (optional)

  • New Feature
  • Bug Fix
  • Other Improvement

Release notes (customer-facing): (optional but encouraged)

  • Secured the local connection between the SDK and the BrowserStack CLI with a per-run token.

Release notes (internal): (required — engineer-facing; what actually changed / why)

  • The SDK generates BROWSERSTACK_CLI_AUTH_TOKEN per binary spawn and sends x-browserstack-cli-token on every RPC; enforced by browserstack-binary#1984, ignored by older binaries. CWE-306 (SDK-7618).

Checklist

  • Ready to review
  • Has it been tested locally?

PR Validations

Run Tests: Comment RUN_TESTS to trigger sanity tests.

🤖 Generated with Claude Code

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 29, 2026 •

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Central YAML (base), Organization UI (inherited), Workspace UI (inherited); Enforced overrides: Organization
  • Review profile: ASSERTIVE
  • Plan: Enterprise
  • Run ID: fe29a192-f3d3-4fa5-a3b8-1ad4e0c0b01d

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • Trigger review
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

CodeRabbit Verify

  • Run CodeRabbit Verify to test the changes in this pull request.

@rounak610

Copy link
Copy Markdown
Collaborator Author

E2E verification (SDK-7618) against a packaged binary built from browserstack-binary#1984 (head b848bf2a), with the binary pointed at via SDK_CLI_BIN_PATH. In each run an unrelated local process called ConnectBinSession on the live binary (no token / wrong token) to check enforcement.

Framework: wdio v9 mocha, maxInstances: 2, Windows 11 chrome.

Run Service Result Outside call
A released 9.42.0 3/3 passed OK (open, as today)
B this PR (npm pack build) 3/3, both workers connected as child processes UNAUTHENTICATED (no token and wrong token)
C this PR + BROWSERSTACK_CLI_AUTH_DISABLE=true 3/3 OK

Unix socket srw-------; gRPC event counts identical across A/B/C; token not in any log.

@rounak610
rounak610 marked this pull request as ready for review October 30, 2026 09:18
@rounak610
rounak610 requested a review from a team as a code owner October 30, 2026 09:18
@github-actions

Copy link
Copy Markdown
Contributor

🔴 SDK PR Review gate is red. Pending:

  • The SDK PR Review Agent has not been run on the current head commit yet — run the SDK PR Review Agent (its verdict is advisory; this gate only requires that it ran on the latest commit).

It turns green once the SDK PR Review Agent has run on the current head commit (any verdict — the gate only requires that the review ran). A native reviewer approval is separately required by branch protection before merge.

@github-actions

Copy link
Copy Markdown
Contributor

🔴 SDK PR Review gate is red. Pending:

  • The SDK PR Review Agent has not been run on the current head commit yet — run the SDK PR Review Agent (its verdict is advisory; this gate only requires that it ran on the latest commit).

It turns green once the SDK PR Review Agent has run on the current head commit (any verdict — the gate only requires that the review ran). A native reviewer approval is separately required by branch protection before merge.

@rounak610

rounak610 commented Nov 2, 2026 •

Copy link
Copy Markdown
Collaborator Author

✅ Good to go

File Status Reason
.changeset/pr-319.md ✅ All Clear Judged clean
packages/browserstack-service/src/cli/grpcClient.ts ✅ All Clear Judged clean
packages/browserstack-service/src/cli/index.ts ✅ All Clear Judged clean
packages/browserstack-service/src/constants.ts ✅ All Clear Judged clean
packages/browserstack-service/src/grpc/index.ts ✅ All Clear Judged clean

Change map (generated deterministically from the diff)

graph LR
  subgraph nwdio_service["wdio-service"]
    npackages_browserstack_service_src_cli_grpcClient_ts["⚠ grpcClient.ts<br/>~13 lines"]
    n_changeset_pr_319_md["pr-319.md<br/>~5 lines"]
    npackages_browserstack_service_src_cli_index_ts["index.ts<br/>~5 lines"]
    npackages_browserstack_service_src_constants_ts["constants.ts<br/>~2 lines"]
    npackages_browserstack_service_src_grpc_index_ts["⚠ index.ts<br/>~2 lines"]
  end
Loading

↻ This verdict comment is the review anchor — it's updated in place on each run (the gate posts its status separately).

— SDK PR Review Agent

@github-actions

github-actions Bot commented Nov 2, 2026

Copy link
Copy Markdown
Contributor

🟢 SDK PR Review gate is green — the SDK PR Review Agent has run on the current head commit (verdict: success).

This gate confirms a review ran on the latest commit. The verdict itself is advisory — read the findings and use your judgement; it does not block merge. A native GitHub reviewer approval is still separately required by branch protection before this PR can merge.

1 similar comment
@github-actions

github-actions Bot commented Nov 2, 2026

Copy link
Copy Markdown
Contributor

🟢 SDK PR Review gate is green — the SDK PR Review Agent has run on the current head commit (verdict: success).

This gate confirms a review ran on the latest commit. The verdict itself is advisory — read the findings and use your judgement; it does not block merge. A native GitHub reviewer approval is still separately required by branch protection before this PR can merge.

pranay-v29
pranay-v29 previously approved these changes Nov 3, 2026
@rounak610
rounak610 requested review from harshit-browserstack and removed request for 07souravkunda and shivam5643 November 3, 2026 11:05
…switch

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

🔴 SDK PR Review gate is red. Pending:

  • The SDK PR Review Agent has not been run on the current head commit yet — run the SDK PR Review Agent (its verdict is advisory; this gate only requires that it ran on the latest commit).

It turns green once the SDK PR Review Agent has run on the current head commit (any verdict — the gate only requires that the review ran). A native reviewer approval is separately required by branch protection before merge.

@rounak610

Copy link
Copy Markdown
Collaborator Author

Removed the BROWSERSTACK_CLI_AUTH_DISABLE off switch (f2bb400). BrowserstackCLI.start() now always sets a fresh randomBytes(32) token before spawning the binary. The changeset no longer mentions the switch.

Re-tested against the browserstack-binary#1984 build with the patched npm pack service (9.42 line, wdio v9 mocha, 2 workers):

Run Result Outside call (no token / wrong token / right token)
Patched 3/3, 2/2 workers connected UNAUTHENTICATED / UNAUTHENTICATED / OK
Patched + BROWSERSTACK_CLI_AUTH_DISABLE=true exported 3/3, 2/2 connected, token still generated UNAUTHENTICATED / UNAUTHENTICATED / OK
  • Socket and token: the socket is srw------- in both runs. The token is not in any log.
  • Static checks: build, tsc --noEmit and eslint on the changed files are clean.
  • Unit tests: tests/cli 473/473, full suite 1463/1463.

@github-actions

github-actions Bot commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

🟢 SDK PR Review gate is green — the SDK PR Review Agent has run on the current head commit (verdict: success).

This gate confirms a review ran on the latest commit. The verdict itself is advisory — read the findings and use your judgement; it does not block merge. A native GitHub reviewer approval is still separately required by branch protection before this PR can merge.

1 similar comment
@github-actions

github-actions Bot commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

🟢 SDK PR Review gate is green — the SDK PR Review Agent has run on the current head commit (verdict: success).

This gate confirms a review ran on the latest commit. The verdict itself is advisory — read the findings and use your judgement; it does not block merge. A native GitHub reviewer approval is still separately required by branch protection before this PR can merge.

@rounak610

Copy link
Copy Markdown
Collaborator Author

RUN_TESTS

@minionhelperappqa

Copy link
Copy Markdown

[SDK Wdio Test] TRA build state: passed | Stability 100% — verdict: success. Passed: 53, Failed: 0, Aggregate: 53. TRA: https://observability.browserstack.com/builds/mtbz6kbktxwpqato3fxxiwfil7xz151mr4d31ioo

@Dalwin-Barnard
Dalwin-Barnard merged commit d447185 into main Nov 6, 2026
28 of 32 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants