Skip to content

Secret storage v2 #1420

Description

@l2ysho

Note

TL;DR — Secrets live under one fixed name per kind: keyring accounts token and proxy-password, or the top level of auth.json on the file backend. A second account would overwrite the first one's token. This keys both backends by userId and re-keys existing secrets. Both backends change in one PR, because a failed keyring write switches to the file backend mid-run.

Part of #1383 (Stage-1). Depends on #1419 — the userId keys come from the v2 file.

Change

Now After
keyring com.apify.cli / token, proxy-password token:<userId>, proxy-password:<userId>
file backend auth.json.token, auth.json.proxy.password inside the matching profile object

One shared secretKey(userId, kind) used by both backends.

Alternative worth one minute of thought: service per kind (com.apify.cli.token with account <userId>) instead of one service and a composite account. It reads better in Keychain Access and other keyring UIs. Check that : in an account name is accepted on macOS Keychain, libsecret and Windows Credential Manager before settling on the composite form.

Why both backends in one PR

downgradeBackendToFile() flips the backend inside a single process when a keyring write fails. If one backend uses the old key scheme and the other the new one at that moment, the downgrade writes the secret under a name the next read does not look for.

Splitting it later is possible with a fallback read on both sides and a test for the mixed state. Decide that with the diff in front of you.

Migration

At the time this ships every user has exactly one profile — Stage-3 (#1384) is what creates a second. So the migration is a single rename, not a loop. That keeps the risk low and the rollback simple.

  • write the new entry, verify it reads back, then delete the old one. The reverse order loses the token if the delete succeeds and the write does not.
  • file backend: move the secret from the top level into the profile object.
  • a keyring write failure keeps the existing downgrade path.
  • idempotent and single-flight, like ensureMigrated().

Decisions

  • No userId in the file. utils.ts:113 already throws "Stale credentials found without user metadata" for this state. Either drop the secret and force a re-login, or keep reading the old key as a fallback. Pick one.
  • macOS Keychain prompt. Creating a new item may prompt, and the migration would run on the first command after an upgrade. Test by hand before release. If it prompts, re-key at next login instead of at first command.

clearKeyringSecrets

The keyring has no listing API, so auth.json is the index of what exists in it. clearKeyringSecrets() must iterate profiles from the file. Delete the keyring entry before removing the profile from the file — a crash between the two otherwise leaves a secret nothing can reach.

Today the function works without the file at all, because the two key names are fixed. Per-profile keys remove that safety net: if a user deletes auth.json by hand, the secrets become unreachable. Keep deleting the legacy token and proxy-password entries as well — one extra call, and it keeps logout working for anyone who never re-keyed.

Supported intermediate state

v2 file with old-style secret keys is valid and will exist for every user between the #1419 release and this one. Both migrations must be independent.

Verification

  • old key → new key on the keyring, secret readable afterwards
  • old top-level secret → profile object on the file backend
  • keyring write fails mid-migration → downgrade to file, secrets intact
  • APIFY_DISABLE_KEYRING=1 toggled between login and logout does not orphan entries
  • logout clears the right entries and leaves other profiles alone
  • apify run still gets the right token and proxy password
  • bundle build still loads the native keyring module
  • the keyring mock in credentials.test.ts:22 keys entries as ${service}:${account}; make sure the new account names do not collide in that map

🤖 Generated with Claude Code

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    t-buildersIssues owned by the Builders team.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions