Conversation
…rovider credentials
…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.
Size Report
Startup median (7 runs, lower is better):
|
|
There was a problem hiding this comment.
All reported issues were addressed across 27 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
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 Not blocking: no test pins that the daemon digests its own startup env at 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 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: 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 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.
|
Addressed in a0ac012.
|
There was a problem hiding this comment.
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, |
There was a problem hiding this comment.
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>
| ...process.env, | |
| ...process.env, | |
| AGENT_DEVICE_HTTP_AUTH_HOOK: '', |
Summary
A local daemon keeps its startup env while
connectchecks 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: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.lease_allocate.29 files, 814 gross lines; help and docs updated.
Validation
At
a0ac012282:pnpm check:affected --runpasses (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 onlyLIMRUN_REGION, or the other platform's variables, also refuses (fails closed, as documented). No live run with real keys.