Skip to content

fix(providers): refuse lease allocation when the daemon holds other provider credentials - #3207

Open
thymikee wants to merge 4 commits into
mainfrom
claude/provider-credential-fingerprint
Open

thymikee wants to merge 4 commits into
mainfrom
claude/provider-credential-fingerprint

Conversation

@thymikee

@thymikee thymikee commented Oct 4, 2026 •

Copy link
Copy Markdown
Member

Summary

A local daemon keeps its startup env while connect checks the shell's, so a stale daemon could act on old credentials, for example creating a billed Limrun instance for an attach-mode shell. Lease allocation now refuses that before anything is created:

Error (INVALID_ARGS): The running daemon holds different limrun credentials than this shell.
hint: Stop it with agent-device daemon stop --state-dir '<dir>', then rerun the command …
  • On a local lease_allocate, the CLI sends a versioned hash of the values each provider's own reader uses (Limrun, BrowserStack), never the values themselves. It goes over the socket and local HTTP. The daemon compares it with the hash of its startup env.
  • No hash is sent from a shell without credentials, or to remote, proxy or cloud daemons. A daemon with an HTTP auth hook treats every caller as remote and skips the check. AWS Device Farm is excluded.
  • The CLI loads the fingerprint module only for lease_allocate.

29 files, 814 gross lines; help and docs updated.

Validation

At a0ac012282: pnpm check:affected --run passes (4,741 related tests, eager-closure budgets, daemon-wire-compat). A startup-level test boots a daemon with credentials and fails if it compares against anything but its own env. Remaining minor: changing only LIMRUN_REGION, or the other platform's variables, also refuses (fails closed, as documented). No live run with real keys.

…toring them in the profile

The client sends a digest of its provider credential variables with lease_allocate to a local
daemon, and the daemon refuses with reason provider-credentials-changed when that digest differs
from the one it started with. This replaces the profile-stored fingerprint.
…r credentials

A client sends no provider credential fingerprint when its environment sets none of the
provider's variables, so a daemon that holds the secrets keeps serving agent shells and Node
workers that have none. A daemon started without credentials still refuses a shell that has them.

The request router now requires the daemon's provider credentials, so daemon composition cannot
drop them, and tests cover the client transport, the HTTP surface, and the router path.
@github-actions

github-actions Bot commented Oct 4, 2026 •

Copy link
Copy Markdown

Size Report

Metric Base Current Diff
Installed (including dependencies) 4.97 MB 4.97 MB +2.1 kB
Package (unpacked) 4.97 MB 4.97 MB +2.1 kB
Package (download) 1.49 MB 1.49 MB +907 B

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 28.6 ms 27.6 ms -1.0 ms
CLI --help 83.8 ms 82.4 ms -1.4 ms

@github-actions

github-actions Bot commented Oct 4, 2026 •

Copy link
Copy Markdown
PR Preview Action v1.8.1

QR code for preview link

🚀 View preview at
https://callstack.github.io/agent-device/pr-preview/pr-3207/

Built to branch gh-pages at 2026-10-04 19:14 UTC.
Preview will be ready when the GitHub Pages deployment is complete.

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

All reported issues were addressed across 27 files

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread src/provider-credential-fingerprint.ts Outdated
Comment thread src/daemon/server/http-server.ts
Comment thread website/docs/docs/browserstack.md Outdated
Comment thread src/provider-credential-fingerprint.ts Outdated
Comment thread src/provider-credential-fingerprint.ts Outdated
Comment thread website/docs/docs/limrun.md Outdated
Comment thread src/commands/schema/cli-help.ts Outdated
Comment thread src/daemon/handlers/lease.ts Outdated
Comment thread test/wire-compat/ledger.json Outdated
@thymikee

thymikee commented Oct 4, 2026

Copy link
Copy Markdown
Member Author

Reviewed 27dd43a. The refusal logic itself looks right, but the Coverage check fails on this diff, so it needs a change before merge.

The eager-closure test in scripts/__tests__/eager-closure-budgets.test.ts fails on two edges this PR adds. providers.ts now imports AppError, which grows the providers subpath from 1 to 3 modules. src/cli.ts now reaches provider-limrun-credentials.ts through daemon-client.ts and the fingerprint module, which takes it from 295 to 297. Please make the gate pass without raising its budgets. The rule: providers.ts stays a leaf that only holds BROWSERSTACK_CREDENTIAL_VARIABLES, and the CLI loads the fingerprint module only for lease_allocate. Moving requireBrowserStackCredentials next to its consumers in provider-definitions.ts and loading the fingerprint lazily in sendToDaemon would do it. The Coverage run should then pass.

Not blocking: no test pins that the daemon digests its own startup env at daemon-runtime.ts:450, so reading {} there would still type-check. One startDaemonRuntime-level assertion would cover it. You can take or leave this.

Could the BrowserStack reader and its trimming rule live in one function that both the consumer and the fingerprint call? That would close the trim thread by construction and keep providers.ts a leaf. I also looked for a smaller owner than the per-request field. Putting the digest in daemon.json and checking it on the stale-daemon path would decide before any RPC. It writes a credential digest to disk and needs a restart policy, so I do not think it is smaller.

On the open threads, two P1s from cubic-dev-ai still apply: trim vs raw key mismatch and auth-hook strip on a local daemon. One P2 still applies: constructor provider lookup throws. Four P3 threads still apply. You can resolve two P2s: remote daemon wording does not apply because the help text says these providers only use local profiles, and fingerprint as authentication does not apply because it is only a staleness check and a caller that sends none already proceeds with the daemon's credentials.

I did not run the eager-closure test locally. I took the failure from the CI log and the diff's import edges. There was no live run with real Limrun or BrowserStack keys. The refusal fires before any provider call, and the socket and HTTP sendToDaemon test covers the client-to-daemon route, so I did not require one. The end-to-end connect limrun plus open refusal under a stale daemon is still unobserved. I also did not check whether a Limrun runtime registers in a daemon started without credentials, which decides whether the "no credentials" refusal or the provider-not-available refusal fires first.

No conflicts. Before merge, the eager-closure gate must pass, and the two held P1 threads need an answer. For the second, either cover the local auth-hook HTTP case or document it.

…ngerprint lazily

- The fingerprint hashes the exact values each provider's own reader uses
  (BrowserStack values as read, Limrun values trimmed), so a credential that
  differs only in whitespace cannot pass as the daemon's.
- providers.ts is a leaf again: it holds the BrowserStack variable names and
  their non-throwing reader; the throwing require moves next to its consumers.
- The client loads the fingerprint module only for a local lease_allocate,
  keeping both eager-closure budgets.
- Provider lookup uses a Map, so inherited object keys are not providers.
- A daemon without a provider's credentials records none and says so in the
  refusal; the hint gives the exact stop command once.
- Document that a daemon with an HTTP auth hook treats every caller as remote.
- Tests: exact-value hashing, inherited keys, a reachable no-credentials case,
  and a startup-level test that the daemon compares against its own env.
@thymikee

thymikee commented Oct 4, 2026

Copy link
Copy Markdown
Member Author

Addressed in a0ac012. pnpm check:affected --run passes locally, including eager-closure-budgets.

  • Eager closure: providers.ts is a leaf again. It holds the BrowserStack variable names and a non-throwing readBrowserStackCredentials, with no imports. requireBrowserStackCredentials moved into provider-definitions.ts and is exported from the package index, which the CLI callers already load. sendToDaemon imports the fingerprint module only for a local lease_allocate, so src/cli.ts is back at its merge-base count.
  • Trim P1: closed by construction, as you suggested. The fingerprint hashes the values each provider's reader returns: BrowserStack's single reader, and Limrun's trimmed values through readLimrunCredentialValues.
  • Auth-hook P1: documented. An auth-hook daemon already treats every HTTP caller as remote (it strips developerDir and refuses path installs) and runs on its operator's credentials, so it skips the comparison. The strip comment, the help rule and the ledger rationale say so.
  • constructor P2: the provider lookup is now a Map.
  • P3s: fixed the docs wording, "rerun the command", the refusal text for a daemon without credentials (it now records none), and the ledger rationale. I replied to and resolved the two P2s you marked as not applicable.
  • Daemon env wiring: a new test in daemon-runtime-interactor-composition.test.ts boots a daemon with BrowserStack credentials and sends lease allocations over HTTP. A mismatch is refused and a match is not. It fails if the daemon reads {}.
  • Reachability: a Limrun daemon without credentials doesn't register the runtime, so the provider-not-available refusal fires first. The "no credentials" lease test therefore uses BrowserStack, which always registers.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

1 issue found across 17 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/daemon/server/daemon-runtime-interactor-composition.test.ts">

<violation number="1" location="src/daemon/server/daemon-runtime-interactor-composition.test.ts:98">
P2: This test inherits `AGENT_DEVICE_HTTP_AUTH_HOOK` from the runner, which makes the HTTP server strip the fingerprint and invalidates the local-daemon comparison it intends to test. Set this variable to an empty string in the test env.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

const daemonEnv = { BROWSERSTACK_USERNAME: 'user', BROWSERSTACK_ACCESS_KEY: 'key-1' };
const runtime = await startDaemonRuntime({
env: {
...process.env,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2: This test inherits AGENT_DEVICE_HTTP_AUTH_HOOK from the runner, which makes the HTTP server strip the fingerprint and invalidates the local-daemon comparison it intends to test. Set this variable to an empty string in the test env.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At src/daemon/server/daemon-runtime-interactor-composition.test.ts, line 98:

<comment>This test inherits `AGENT_DEVICE_HTTP_AUTH_HOOK` from the runner, which makes the HTTP server strip the fingerprint and invalidates the local-daemon comparison it intends to test. Set this variable to an empty string in the test env.</comment>

<file context>
@@ -89,6 +90,55 @@ test('daemon startup composes the interactor resolution the daemon resolves thro
+  const daemonEnv = { BROWSERSTACK_USERNAME: 'user', BROWSERSTACK_ACCESS_KEY: 'key-1' };
+  const runtime = await startDaemonRuntime({
+    env: {
+      ...process.env,
+      ...daemonEnv,
+      AGENT_DEVICE_STATE_DIR: stateDir,
</file context>
Suggested change
...process.env,
...process.env,
AGENT_DEVICE_HTTP_AUTH_HOOK: '',

This branch has not been deployed

No deployments
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.

1 participant