Skip to content

Simplify Compose control-plane authentication - #38

Merged
nathan-thillairajah merged 10 commits into
mainfrom
agent/compose-direct-session
Aug 18, 2026
Merged

Simplify Compose control-plane authentication#38
nathan-thillairajah merged 10 commits into
mainfrom
agent/compose-direct-session

Conversation

@nathan-thillairajah

@nathan-thillairajah nathan-thillairajah commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Summary

bb apps previously exchanged the normal opaque bbidentity session for a Compose-specific purpose token, then cached, refreshed, and retried requests with that token. The new contract moves that exchange to public ingress: while the request is paused, ingress sends the bbidentity session to the authorizer and replaces it with a short-lived Compose JWT before forwarding.

The CLI now sends Authorization: BBIdentity <credential> only to the exact staging and production Compose control-plane hosts over HTTPS. Redirects stay disabled, cookies and legacy identity headers are omitted, and reflected credentials are redacted from errors. Apps commands require an already-valid stored session in every mode; otherwise they fail promptly with auth_required and direct the caller to explicit bb auth login. Only the explicit auth command owns the browser login flow. The old exchange, cache, refresh, and retry machinery is removed.

The staging infrastructure still needs to be activated before this can be smoke-tested end to end. Merging this PR also does not update an installed bb binary: Berd ships the CLI through its normal pinned release process.

Related issue

N/A. This replaces auth work from the archived pre-OSS repository.

Testing

  • cd bb-cli && source ./bin/activate-hermit && just check
  • cd bb-cli && source ./bin/activate-hermit && just build-bb-release

@nathan-thillairajah
nathan-thillairajah force-pushed the agent/compose-direct-session branch from ba31e12 to 1be4c68 Compare August 17, 2026 19:23
@nathan-thillairajah
nathan-thillairajah marked this pull request as ready for review August 17, 2026 19:23
@nathan-thillairajah
nathan-thillairajah requested a review from a team August 17, 2026 19:23

@morgmart morgmart left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Automated code review

REQUEST_CHANGES. Fresh static review of the exact three-dot range found 3 blocking findings and 0 non-blocking findings. The shared authentication path can wait forever when embedded in an Apps command, Apps independently rejects credentials already accepted by the authoritative auth service, and the prescribed recovery from a Compose 401 reuses the same rejected credential. Final self-check covered contract/create/deploy flows; CLI accessibility and localization applicability; allowlist, redirect, credential-consent, and navigation guards; async, failure, never-completes, lifecycle, and race behavior; test honesty; project rules; duplicate overlap; and blocking evidence/user effect. No frontend UI changed, so visual accessibility, responsive design, and design-system token checks are not applicable. Supplied GitHub evidence was inspected: it is structurally available, reports no check runs and a pending combined status; required checks still independently govern merge readiness.

Deterministic publication result: 3 blocking and 0 non-blocking finding(s) publishable; 0 duplicate(s) suppressed.

Comment thread bb-cli/src/bb/apps.rs Outdated
Comment thread bb-cli/src/bb/apps.rs Outdated
Comment thread bb-cli/src/bb/apps.rs

@morgmart morgmart left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Automated code review

REQUEST_CHANGES. A fresh static review of the exact three-dot comparison found 2 blocking findings and 0 non-blocking findings. The noninteractive authentication path verifies one stored credential and then re-reads storage before sending, allowing a concurrent change to substitute an unverified credential. The PR also removes all successful process-level contract/create/deploy coverage while changing the authentication and dispatch path for each command; replacement unit tests bypass production destination validation and do not preserve create orchestration or full CLI wiring. The three previously automated issues were re-evaluated against current code and are fixed: embedded waits are bounded with prompt suppression for JSON/non-stdin-TTY execution, feature-local credential format restrictions were removed, and 401 guidance now prescribes logout before login. Final self-check covered contract, create/initialize, create-without-initialize, and deploy flows; CLI accessibility and localization applicability; destination, redirect, consent, and interaction guards; async, timeout, error, never-completes, lifecycle, and race behavior; test honesty; project rules; duplicate overlap; and concrete evidence plus user effect for blocking severity. No frontend UI changed, so responsive layout, visual accessibility, screenshots, and design-system tokens are not applicable. Supplied GitHub evidence and all three supplied resolved threads were inspected. Evidence is structurally available, reports no completed check runs and a pending combined status; required checks still independently govern merge readiness.

Deterministic publication result: 2 blocking and 0 non-blocking finding(s) publishable; 0 duplicate(s) suppressed.

Comment thread bb-cli/src/bb/apps.rs Outdated
Comment thread bb-cli/tests/bb_e2e.rs

@morgmart morgmart left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Automated code review

REQUEST_CHANGES. A fresh static review of the exact three-dot comparison found 2 blocking findings and 0 non-blocking findings. Ordinary debug builds now contain an environment-controlled transport override that can send the long-lived BBIdentity credential to an unapproved loopback process after validating a different approved URL. The revised login flow also persists a newly issued credential before validating that the response has an active workspace, so this failure leaves a session installed that causes subsequent login attempts to fail before reopening the browser. All five supplied resolved automated threads were re-evaluated against current code; their underlying issues are fixed and no current finding duplicates them. Final self-check covered contract, create/initialize, create-without-initialize, deploy, login, status, logout, and legacy-cache flows; CLI/browser accessibility and localization applicability; destination, redirect, navigation, consent, and interaction guards; async, timeout, error, never-completes, lifecycle, and race behavior; test honesty; project rules; duplicate overlap; and concrete evidence plus user effect for blocking severity. No frontend product UI changed, so responsive layout, application design-system tokens, and PR screenshots are not applicable. Supplied GitHub evidence and all five supplied threads were inspected. Evidence is structurally available, reports no completed check runs and a pending combined status; required checks still independently govern merge readiness.

Deterministic publication result: 2 blocking and 0 non-blocking finding(s) publishable; 0 duplicate(s) suppressed.

Comment thread bb-cli/src/bb/apps.rs Outdated
Comment thread bb-cli/src/bb/auth_login.rs Outdated

@morgmart morgmart left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Automated code review

REQUEST_CHANGES. A fresh static review of the exact three-dot comparison found 2 blocking findings and 0 non-blocking findings. Apps still classifies ordinary stdout pipelines as interactive because it checks only stdin, so a JSON-producing command piped to another program can unexpectedly open a browser and wait five minutes. In addition, the successful built-binary contract/create/deploy tests were removed again; replacement module tests bypass production configuration, session verification, dispatch, and the final production HTTP executor. The seven supplied resolved automated threads were re-evaluated against current code. The credential recipient override, credential-selection race, local format restriction, ineffective 401 guidance, failed-login persistence ordering, and unbounded callback wait are fixed. The successful CLI coverage issue and the broader embedded-interaction issue remain present in their current forms; because their resolved threads contain no substantive human replies, they are eligible to be raised again under the supplied duplicate policy. Final self-check covered contract, create/initialize, create-without-initialize, deploy, login, status, logout, and legacy-cache flows; CLI/browser accessibility and localization applicability; destination, redirect, navigation, consent, and interaction guards; async, timeout, error, never-completes, lifecycle, and race behavior; test honesty; project rules; duplicate overlap; and concrete evidence plus user effect for blocking severity. No frontend product UI changed, so responsive layout, application design-system tokens, and screenshots are not applicable. Supplied GitHub evidence and all seven supplied threads were inspected. Evidence is structurally available, reports no completed check runs and a pending combined status; required checks still independently govern merge readiness.

Deterministic publication result: 2 blocking and 0 non-blocking finding(s) publishable; 0 duplicate(s) suppressed.

Comment thread bb-cli/src/bb/apps.rs Outdated
Comment thread bb-cli/tests/bb_e2e.rs

@morgmart morgmart left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Automated code review

REQUEST_CHANGES. A fresh static review of the exact three-dot comparison found 2 blocking findings and 0 non-blocking findings. The new apps-e2e-test Cargo feature can be compiled into a distributable bb binary; in that configuration an environment variable, embedded test CA, and checked-in matching private key let a loopback process impersonate either approved host and receive the reusable BBIdentity session. Also, successful control-plane responses are returned and printed without credential redaction, so an approved ingress that reflects the Authorization secret in a 200 JSON response can leak it to stdout. All nine supplied resolved automated threads were re-evaluated against current code. Their underlying issues are fixed: Apps is now always noninteractive, exact credential selection avoids a second storage read, failed workspace validation precedes persistence, URL validation remains exact, and built-binary success coverage has been restored. Neither current finding duplicates those threads. Final self-check covered contract, create/initialize, create-without-initialize, deploy, login, status, logout, legacy-cache, and test-feature flows; CLI/browser accessibility and localization applicability; destination, TLS, redirect, navigation, consent, and interaction guards; async, timeout, error, never-completes, lifecycle, and race behavior; successful and failing response redaction; test honesty; design-system/project rules; duplicate overlap; and concrete evidence plus user effect for blocking severity. No frontend product UI changed, so responsive layout, application design-system tokens, and screenshots are not applicable. Supplied GitHub evidence and all nine supplied threads were inspected. Evidence is structurally available, reports no completed check runs and a pending combined status; required checks still independently govern merge readiness.

Deterministic publication result: 2 blocking and 0 non-blocking finding(s) publishable; 0 duplicate(s) suppressed.

Comment thread bb-cli/src/bb/apps.rs Outdated
Comment thread bb-cli/src/bb/apps.rs

@morgmart morgmart left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Automated code review

REQUEST_CHANGES. A fresh static review of the exact three-dot comparison found 2 blocking findings and 0 non-blocking findings. The cfg(test) process helper accepts an arbitrary BB_APPS_E2E_CONTROL_PLANE_URL and rewrites a credential-bearing approved request to that destination without enforcing loopback or isolated auth storage, so a crafted test invocation can exfiltrate a developer or CI session. Separately, recursive successful-response redaction rebuilds JSON objects using redacted keys; distinct original keys can collapse to the same sanitized key and silently overwrite a field. All eleven supplied resolved automated threads were re-evaluated against current code. Their underlying issues are fixed: the distributable bb target no longer includes the feature-gated DNS/CA bypass, Apps is noninteractive, credential selection avoids a second read, failed workspace validation precedes persistence, successful built-binary flow coverage exists in the test executable, and successful response strings are now redacted. Neither current finding duplicates those threads. Final self-check covered contract, create/initialize, create-without-initialize, deploy, login, status, logout, legacy-cache, and cfg(test) process-helper flows; CLI/browser accessibility and localization applicability; destination, redirect, navigation, consent, and interaction guards; async, timeout, error, never-completes, lifecycle, and race behavior; success/error redaction and response-shape integrity; test honesty; project rules; duplicate overlap; and concrete evidence plus user effect for blocking severity. No frontend product UI changed, so responsive layout, application design-system tokens, and screenshots are not applicable. Supplied GitHub evidence and all eleven supplied threads were inspected. Evidence is structurally available, reports no completed check runs and a pending combined status; required checks still independently govern merge readiness.

Deterministic publication result: 2 blocking and 0 non-blocking finding(s) publishable; 0 duplicate(s) suppressed.

Comment thread bb-cli/src/bb/apps.rs
Comment thread bb-cli/src/bb/apps.rs Outdated

@morgmart morgmart left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Automated code review

APPROVE. A fresh static review of the exact three-dot comparison found no publishable blocking or non-blocking findings. All thirteen supplied resolved automated threads were re-evaluated against current code and their underlying issues are fixed: Apps never starts implicit browser authentication; verification and transmission use the same credential instance; failed workspace validation occurs before persistence; 401 recovery replaces the rejected session; the destination is exact-host HTTPS with redirects disabled; no test transport is present in the distributable bb target; the cfg(test) process helper validates explicit-port loopback IP origins and creates isolated synthetic auth storage; successful built-binary-equivalent process flows cover contract, both create branches, and deploy; successful response string values are recursively redacted; and secret-bearing object keys are rejected without mutating response shape. Final self-check covered contract, create/initialize, create-without-initialize, deploy, login, status, logout, legacy-cache, and cfg(test) process-helper flows; accessibility and localization applicability; navigation, recipient, redirect, consent, and interaction guards; async, timeout, error, never-completes, lifecycle, and race behavior; success/error redaction; test honesty; design-system/project rules; duplicate and overlap handling; and evidence plus user effect for blocking severity. No frontend product UI changed, so responsive layout, application design-system tokens, and screenshots are not applicable. Supplied GitHub evidence and all thirteen threads were inspected. Evidence is structurally available, reports no completed check runs and a pending combined status; required checks still independently govern merge readiness.

Deterministic publication result: 0 blocking and 0 non-blocking finding(s) publishable; 0 duplicate(s) suppressed.

Pending checks: 1 check(s) are not complete.

This approval reflects the completed code review only; merge readiness remains governed by the repository's required checks.

@nathan-thillairajah
nathan-thillairajah merged commit fc0cced into main Aug 18, 2026
8 checks passed
@nathan-thillairajah
nathan-thillairajah deleted the agent/compose-direct-session branch August 18, 2026 15:23
nathan-thillairajah added a commit that referenced this pull request Aug 18, 2026
## Summary

The Compose authentication change in #38 changed the `bb` binary, and
the bb release contract requires a version update before Berd packages
it. This bumps the shared bb-cli package and lockfile from 0.7.11 to
0.7.12 so the next installed Berd release carries an identifiable CLI
version.

### Related issue

Follow-up to #38.

### Testing

- `cd bb-cli && source ./bin/activate-hermit && just check`
- `just build-bb-release`; `bb --version` reports 0.7.12
- `scripts/prepare-bb-cli-resource.sh`; the staged resource reports
0.7.12
- Repository pre-commit and pre-push gates
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants