Skip to content

Keep the secrets the first start minted, so a second start can still read the vault - #415

Open
zopeVaibhav wants to merge 1 commit into
CopilotKit:mainfrom
zopeVaibhav:fix/desktop-rotates-key-encryption-key
Open

Keep the secrets the first start minted, so a second start can still read the vault#415
zopeVaibhav wants to merge 1 commit into
CopilotKit:mainfrom
zopeVaibhav:fix/desktop-rotates-key-encryption-key

Conversation

@zopeVaibhav

Copy link
Copy Markdown
Contributor

What this changes

The desktop shell generated all six of its deployment secrets on every press of Start. Five of them
can be rotated harmlessly, because the processes that share them restart together from the same
file. KEY_ENCRYPTION_KEY cannot: it encrypts the credential vault, stack::down runs compose down without -v, and the volume holding the vault outlives the key that opened it. The second
session of an installed OpenBot therefore met credentials it could no longer decrypt, and the error
that came back was OperationError: The operation failed for an operation-specific reason — AES-GCM
refusing a tag, naming neither the key, nor the credential, nor the restart.

COMPUTER_TOKEN is the second one that matters, for a different reason. The supervisor bakes it into
a Bot's computer container as an environment variable at create time (supervisor/src/index.ts:96),
Stop leaves those containers stopped rather than removed so the Bot keeps its volumes
(stack.rs:154), and ensure starts an existing owned container and otherwise leaves it "exactly as
it is" (supervisor/src/docker.ts:422). Those containers are not Compose services, so stack::up
naming every service does not recreate them. After a restart the server held a token no earlier
computer had.

write now keeps the secrets an existing .env already carries and generates only what is not
already there. Two values are deliberately not carried: anything published in this repository — the
.env.example key and start.sh's three openbot-dev-* fallbacks — and a KEY_ENCRYPTION_KEY that
is not a base64 32-byte value, which server/src/config.ts refuses to start on. Carrying either
would keep an install broken in a way nothing on screen could undo.

scripts/start.sh already works this way for the two secrets it generates: MANAGED_AGENT_TOKEN and
AGENT_TOOL_TOKEN are generated once, written back to .env, and reused on later runs. This is that
rule applied to the shell, and to all six.

The guard lives in write rather than in compose because write is already the function that
reads the existing file and decides, per line, what to keep.

Fixes #414.

Where it runs

This runs on one laptop, in a Tauri shell, before any server process exists. It is a desktop
installer writing a file, not a request path.

  • New state that outlives a request? None. The state involved is a file on disk, .env,
    which is the thing being made to outlive a restart rather than new state being introduced.
  • What happens on the second replica? Nothing. This code is not deployed; it is the desktop
    shell that raises a single-machine deployment, and OPENBOT_SINGLE_USER=true is written two
    lines away. A hosted OpenBot never runs it. The failure it fixes is the desktop analogue of a
    replica bug — one process regenerating a secret another process's data depends on — but the
    second process here is the next launch of the same one.
  • Anything serialised? Nothing new. Two copies of the shell cannot run: main.rs hands the
    window over to the instance that already exists.
  • Anything fanned out to a browser? No.
  • New listener, port, or schedule? None. Ports are unchanged, and the test
    everything_that_is_not_a_secret_still_takes_this_run_s_value pins that a port moved between
    versions is still written fresh rather than carried.

Boundary and audit

  • Every acting call still goes through the gateway: unchanged, and not reached by this. No
    server code is touched.
  • New refusals and new failures each write a row: no new refusal or failure exists. The shell
    has no audit trail and predates the server it starts.
  • Nothing new is trusted from the client that the server can resolve itself: nothing new is
    trusted at all. The one new input is the deployment's own .env, and the two ways it can lie —
    a published value, or a key the server would refuse — are both rejected rather than carried.

Changelog

  • A line in CHANGELOG.md under Unreleased.

Proof

Every command .github/workflows/desktop.yml runs on this tree, on this machine:

cargo fmt --check                      clean
cargo clippy --all-targets -- -D warnings   clean
cargo test --lib                       78 passed; 0 failed

The bug reproduces, and the tests catch it. With the carry-forward line removed and everything else
in this branch left in place, three of the new tests fail and the rest still pass:

test env::tests::restarting_keeps_every_secret_the_first_start_minted ... FAILED
test env::tests::a_restart_that_re_mints_the_key_would_leave_the_vault_unreadable ... FAILED
test env::tests::a_secret_somebody_set_by_hand_is_the_one_that_is_kept ... FAILED
test result: FAILED. 19 passed; 3 failed

Restored: 22 passed; 0 failed for env::, 78 passed; 0 failed for the crate.

The consequence was checked against the real code rather than argued. Encrypting under the key the
first Start wrote and decrypting under the key the second Start wrote, through
server/src/credentials.ts unmodified:

with the key it was stored under: sk-a-stored-credential
with the re-minted key: OperationError | The operation failed for an operation-specific reason

Nothing outside desktop/ changed, and the repository is unaffected: bunx biome lint --error-on-warnings . clean over 573 files, bun run typecheck clean in app, server and
worker. bun test gives 2443 pass, 23 skip, 3 fail on this branch and the identical
2443 pass, 23 skip, 3 fail on main — the three are the handoff-queue lease tests, which fail on a
clean checkout and are not touched here.

Not covered: the shell was not installed and driven end to end for this, because reproducing the
original failure that way means installing a container engine, letting the first Start pull five
images, storing a credential through the UI, quitting and starting again. The two halves were each
run instead — the file the shell writes, in Rust, and the decryption the server does, in TypeScript —
and they meet at the value of KEY_ENCRYPTION_KEY. desktop.yml says the core job is "the only
job that runs the assertions about what the shell writes into .env", which is the level this is
tested at.

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.

Starting the desktop app a second time re-mints KEY_ENCRYPTION_KEY, so stored credentials stop being readable

1 participant