Repository navigation
feat: auth.json v2 with profiles keyed by user ID - #1434
Merged
Merged
Conversation
l2ysho
force-pushed
the
claude/auth-json-v2-1419
branch
from
September 17, 2026 11:46
7a941e1 to
cc85096
Compare
auth.json was a flat blob describing one account, holding the whole
user('me') response. It is now { version, activeProfile, profiles,
secretsBackend }, so it can hold N accounts. Nothing puts a second one
there yet, and users see no change.
New src/lib/auth-file.ts owns the file: reading, an atomic write, the
v1 to v2 migration, and the profile accessors. credentials.ts, login,
logout, getLocalUserInfo() and the rental notice all go through it.
The migration backs the old file up as auth.json.v1.bak, runs after
ensureMigrated() as a separate step, is idempotent and single-flight,
and never throws. Fields nothing reads are dropped: email, plan,
effectivePlatformFeatures, isPaying, createdAt and proxy.groups.
Closes #1419
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Both parsed auth.json by hand and asserted the v1 flat shape, so neither
could pass against a v2 file. log_in_out deep-equalled the file against
the whole user('me') response, which v2 deliberately no longer stores;
info read a top-level id that is now the profile key.
Both now read the active profile through the test helper, and
log_in_out checks the token through getToken() rather than the file.
Not run here — test:api needs a live token.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
copyFileSync inherits the source mode. An auth.json written before the CLI started passing mode 0600 is still 0644, and writeFileSync's mode applies only on create, so it stayed that way. The new atomic write fixes auth.json on the first v2 write, but the backup is copied before that and never rewritten — leaving a plaintext token at 0644. Also fixes two tests: apify info prints three rows since the token source line landed, and the idempotency check called the migration twice without resetting the memoised promise, so the second call never touched the file. Adds the missing cover for logout removing the backup, which is the only path that erases that token from disk. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The backup is written once and never refreshed, and only logout deletes it. So after `apify login` as a second account, auth.json holds the new token while auth.json.v1.bak still holds the previous one — for as long as the user never logs out. Nothing reads the backup, and a downgraded CLI finds its token through the keyring or auth.json rather than here, so the secrets are dropped when writing it. Also pins the two lines that make the migration run for users. Deleting `await ensureAuthFileCurrent()` from either resolveAuth() or getLocalUserInfo() left the whole suite green: every migration test called it by hand. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
l2ysho
force-pushed
the
claude/auth-json-v2-1419
branch
from
September 23, 2026 09:14
d903722 to
0452abf
Compare
Windows has no POSIX modes. Node reports 0o666 and chmod only moves the read-only bit, so the assertion read 438 where it wanted 384. The two other mode tests in the suite already skip on win32; this one now matches them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Moving data between shapes later needed this module reworked: the
migration was one function gated on "no version field", so the next
format change had nowhere to go. It is now a table keyed by the version
each step upgrades from, and a file runs every step from its own version
upwards. Adding a step is an entry in the table.
A failed migration now says so once instead of only under APIFY_DEBUG.
It still never blocks a command, because the readers understand the old
shape, but failing on every run should be visible.
Other fixes from review:
- Reserve AuthProfile.secretsBackend. Once secrets are keyed per
profile, a keyring failure on one profile must not silently redirect
another profile's reads to the file backend.
- ensureMigrated() skips a file a newer CLI wrote. It runs before the
shape migration reports the version, and would otherwise rewrite it.
- Write the backup through the atomic writer, the one plain write left
in a module built around temp file plus rename.
- Treat a non-object JSON payload as unusable. JSON.parse('"abc"')
succeeds and Object.keys('abc') is ['0','1','2'], so it passed both
migration guards and got replaced.
- Drop the comment calling the backup a way back. Nothing reads it and
no procedure restores it; the comment beside it already said so.
Tests:
- getLocalUserInfo() returns organizationOwnerUserId. Deleting that line
left the suite green while demoting every organization login to a
personal account.
- A pre-existing 0644 auth.json is tightened to 0600. Only temp file
plus rename does that; writeFileSync's mode applies on create only.
- The three apify run tests read the token and proxy password from disk
again. They had come to assert getLocalUserInfo() against itself.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
setActiveProfile(userId, profile, backend) read as "mark this one active". It means "make this the only account, and drop the previous one's secrets". It is now replaceStoredAccount, and the docblock says why it replaces rather than adds. Replacing is deliberate twice over. Until each profile has its own secret, a second profile would name an account that cannot authenticate. And dropping the old secrets is what makes the write safe: loginWithToken writes the new token straight after, so a failure there leaves no token at all rather than the previous account's token sitting beside the new account's name. Two tests pin that second half, which nothing covered. Making the write preserve siblings and root secrets — the shape Stage-2 will need — fails both: the old profile survives, and so does the old token. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
AuthFile carried `[k: string]: unknown`, so it typed the v1 shape, the v2 shape and an empty object identically. Reading a field that no longer exists stayed legal, which matters because #1420 moves the token and proxy password off the top level and into the profile. Measured: remove `token` from the type and the old signature reported 2 errors. It now reports 12 — every reader, across auth-file.ts and credentials.ts. Ten sites would have gone unnamed. The v1 fields move to LegacyAuthFile, which extends AuthFile with the three the flat shape carried. Only v1Profile, toV2 and the fallback in lookUpActiveProfile take it; that fallback is the one cast left, and it is where a file this CLI cannot version lands. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The version guard ran before resolveAuth read APIFY_TOKEN, so a stored file written by a newer CLI stopped every command — including ones that never read that file. A platform run or a CI job has APIFY_TOKEN as its only credential and no interest in the stored login, and `apify run` calls resolveAuth uncaught while deliberately catching the account lookup on the next line. The guard now runs on the stored-login path only. Logout was refused by the same guard, which left no way out of the state: the error offered "Upgrade the CLI" and never mentioned the file. Logout exists to discard credentials, so it no longer checks the version. A shape this CLI cannot read is discarded whole rather than edited — the old code deleted fields from it and wrote it back, which left a mangled file when it held more than one profile. The error names `apify logout` as the escape, and drops the parenthetical aside the repo's copy style does not take. Removing the auth files also passes maxRetries again. rimrafPromised carried 10 retries against Windows EBUSY when an antivirus or a second process holds the file; the bare rmSync that replaced it had none. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Three kinds went: - Restating the signature. "The parsed file, or an empty object when it is missing or unreadable" above a function that returns exactly that. - Narrating the branch. "No index signature: removing one names every reader at compile time" justifies a commit, in a place that will rot once nobody remembers there was one. - Repeated verbatim. The same three-line rationale sat above all three apify run assertions; one earns its keep. Three fields carrying the same "unused until the device flow lands" share one line now, and two of the longer blocks say the same thing shorter. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
"Could not update auth.json to the current format, so it was left as it is" reads like something broke. Nothing did — the readers understand the old shape, so the command that triggered it carries on and the next one tries again. A user with a read-only ~/.apify saw an alarming line on every command with no action to take. The message now leads with what matters to them and names the debug variable for the part that does not. Adds the failure-path test. The whole migration sits in one try/catch and nothing covered it: removing the warning left the suite green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two were wrong. ensureAuthFileCurrent said it brings the file "to the v2 profile shape", which predates the step chain, and that migrating "never throws" — the function does, through the version assert one line below. lookUpActiveProfile described the pre-profile read path as something that covers commands running before the migration, which reads like scaffolding; useRentalSunsetNotice calls it without migrating on purpose, so that path is permanent. The rest were restating the signature, saying the same thing in two docblocks, or taking five lines for one idea. Comment-only: the diff has no non-comment lines. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
l2ysho
marked this pull request as ready for review
September 24, 2026 10:52
auth.json.v1.bak described the account it was taken from and is never refreshed, so only logout removed it. Log in as someone else and one user's details sat on disk under another user's login. A login now discards it. Profiles carry loggedInAt. Nothing reads it — `auth list` will order by it, and a logout will fall back to the most recent profile left — but neither can backfill a time nobody recorded, so it has to be written from the start. A profile migrated from the pre-profile file gets null, because that file never held one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Four lines for a field whose name and type say it. The reason it exists belongs in the commit that added it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This was referenced Sep 24, 2026
DaveHanns
approved these changes
Oct 1, 2026
DaveHanns
left a comment
Contributor
There was a problem hiding this comment.
Good job 🚀
I have also let Claude go through the PR and it found few nits. Feel free to ignore.
Should be addressed
- PR body vs #1383: the body says an old CLI reads the v2 file and no release note is needed. Not quite. 1.10.0
loginon a v2 file leavesactiveProfilepointing at the old account while the token is the new one, and 1.10.0runthrows on a fresh v2 file until an API command rewrites the flat fields. Either mirror flatid/usernameon v2 writes, or add the release note. auth.json.v1.bak: nothing reads it, it can't restore a login without secrets, it keepsemail, and a 1.10.0 logout orphans it. Drop it or document what it's for.logout.ts:31-33:removeActiveProfilenever refuses, so the comment is wrong. The order also flipped to file-first, and #1420 needs keyring-first. Keep the original order.
Nits
auth-file.ts:19-32: reserved fields are required but never read. Make them optional. Body says per-profilesecretsBackendis written; it isn't.auth-file.ts:94-106: third copy of temp-plus-rename. Lift intofiles.ts, can be a follow-up.auth-file.ts:177,credentials.ts:253:typeof version === 'number'treats a string"2"as v1 and dropsprofiles.auth-file.ts:231: drop theuseRentalSunsetNoticereference, the hook already explains it.auth-file.ts:256-261,auth.ts:195: "drops the previous secrets" is true on the file backend only. On keyring the old token lives untilsetTokenoverwrites it.auth-file.ts:267, 306-307:rmSyncignoresmaxRetries/retryDelaywithoutrecursive: true.types.ts:3:AuthJSONis now the resolved view, not the file. Doc comment for now.gitSource.ts:100"asauth.jsonrecords it" is stale.auth-file.test.ts:291, 328: duplicatedescribe('replacing the stored account').auth-file.test.ts:183: fails as root. Addprocess.getuid?.() === 0to the skip.auth-file.test.ts:180: comment describes review history, not the test.credentials.test.ts:40:authFileWrites()also counts.v1.bak.tmp-*writes.
auth.json.v1.bak was written so an upgrade stayed inspectable. Nothing reads it, it cannot restore a login since the secrets were taken out of it, it keeps the account's email, and a 1.10.0 logout removes auth.json without knowing about it, so it is left orphaned. It carries the cost of a credential file and none of the benefit. No released build ever wrote one — auth-file.ts is not on master — so there is nothing on anyone's disk to clean up. Logout clears the keyring first again. The comment said the file step goes first because it is the one that can refuse, which stopped being true when removeActiveProfile lost its version guard. #1420 needs the keyring cleared while the profile that names its entries still exists. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
l2ysho
added a commit
that referenced
this pull request
Oct 8, 2026
…unt-ux-1384 Resolves the conflicts with #1434 dropping the v1 backup: removeAllProfiles calls discardAuthFile, and the backup test goes with the backup. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
l2ysho
added a commit
that referenced
this pull request
Oct 8, 2026
> [!NOTE] > **TL;DR** — > - automatic migration to v2 shape of auth.json > - structure ready for multiple accounts -> #1458 > - user shouldn't notice anything, no re-login needed Stacked on #1434 (Stage-1, subtask 4). Base branch is `claude/auth-json-v2-1419`, not `master`. Closes #1420. Part of #1383. ## What changes for users Nothing in normal use. Same commands, same output. Stored logins move to the new names on the first command after the upgrade, with nothing to run or re-enter. Three differences, all on failure paths: - `logout` used to exit 0 and say the keyring was cleared even when it was not. It now exits **1** and names the entry left behind, by `service/account`. - `login` warns when it could not remove a secret it meant to replace. The proxy-password case was silent before, and the stale password kept being injected into every local run. - A keyring that cannot answer at all still reports nothing, so a machine with no secret service is unaffected. **One contract change worth a release note:** on a machine where the keyring exists but refuses a delete, `logout` now fails loudly. `apify logout && …` stops there instead of continuing. This is also what makes more than one account storable. Nothing here exposes that — no `--profile`, no `auth switch`. ## Change | | Before | After | |---|---|---| | keyring | `com.apify.cli` / `token`, `proxy-password` | `com.apify.cli.token`, `com.apify.cli.proxy-password` / `<userId>` | | file | `auth.json.token`, `auth.json.proxy.password` | inside `profiles[<userId>]` | **Service per kind, not a composite account.** `token:<userId>` under one service would depend on `:` being legal in an account name on macOS Keychain, libsecret and Windows Credential Manager. `getToken` / `setToken` / `getProxyPassword` / `setProxyPassword` / `deleteProxyPassword` collapse into `getSecret(userId, kind)`, `setSecret(userId, kind, value)` and `deleteSecret(userId, kind)`. **Both backends in one PR**, because a keyring write can fail and send that account to the file mid-run. Split across two releases, that fallback would write the secret under a name the next read does not look for. ## Which backend an account is on `backendFor(userId)` answers from the token alone: a token in `auth.json` means the file, otherwise the keyring unless it is disabled or will not load. No marker is persisted, so one keyring failure never pins a later write to the file. When a token write falls back to the file, the account's other secrets move with it — reads for that account all go to the file from that point. ## Migration `ensureCredentialsCurrent()` runs the three steps in their one valid order: plaintext secrets into the keyring, the file into its current shape, then the secrets onto keyed names. Every reader calls it. - Write the new entry, verify it reads back, then delete the old one. - Nothing the account already has is overwritten, and nothing under the fixed names is claimed for an account that already has a keyed token. - File backend: one atomic write moves the secrets into the profile and clears the top level. - Idempotent, single-flight, never throws. `loginWithToken()` clears the fixed names on every login, including a repeat of the same account. **A v1 file with a token but no `id`** has no key to file the secret under. The secret is dropped and the next command asks for a re-login — that state already required one. **A hand-deleted `auth.json` orphans keyed entries.** The CLI never enumerates the keyring, so the file is its only index of what it holds. `findCredentials()` would enumerate a service on every platform but the Linux keyutils fallback, so a repair path stays open. ## Verification - `pnpm run test:local` — **691 passed, 4 skipped (63 files)**. - `pnpm run lint`, `pnpm run format`, `pnpm run build` — clean. - `pnpm run update-docs` — no change; no flag, arg, description or registration moved. - `pnpm run test:api` — green in CI. - **Install size unchanged** — no dependency added or removed. - Covered: old name → keyed name on the keyring; top-level secret → profile on the file; a keyring failure moving one account to the file while a second keeps reading the keyring; a login that runs before the migration; one account not claiming another's secret; a keyring that refuses a delete, and one that answers nothing at all; `APIFY_DISABLE_KEYRING` toggled between login and logout. - Tests now mock `@napi-rs/keyring` globally. `__APIFY_INTERNAL_TEST_AUTH_PATH__` relocates `auth.json` but not the OS keyring, and two test files that run `logout` were reaching the developer's own stored login. **Manual, on macOS.** Login and logout on both backends, the migration off the fixed names, and a login landing before the migration. A real Keychain ACL denial was hit by accident, which exercised the failure path end to end: `logout` exited 1, named `com.apify.cli/proxy-password`, left the entry in place and removed the profile. **Not verified: whether creating an item under a new service prompts on macOS.** The migration runs on the first command after the upgrade, so if it prompts, re-key at next login instead. ## Known and deferred - `ensureSecretsKeyed()` has no terminating condition, so it probes the two fixed names on every command — 2 of 4 keyring reads, unchanged from the start of this branch. On a machine where those entries cannot be deleted, it never stops. - The same fix covers both guards above: `MIGRATION_STEPS` in `auth-file.ts` is `(file) => file`, synchronous and pure, so keying could not be a versioned step. Widening it and bumping `AUTH_FILE_VERSION` to 3 retires the guards, the probe, and the file-shape helpers still in `credentials.ts`. - `test/` is not typechecked (`tsconfig.json` is `"include": ["src"]`), which is how the `test:api` imports broke unnoticed. ## Left out - No additive login, no `--profile`, no `auth switch`, no `auth list`. - `clearKeyringSecrets()` takes the profile whose entries to clear, but only ever sees one profile today — Stage-2 (#1386) is what puts a second one there. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
l2ysho
added a commit
that referenced
this pull request
Oct 8, 2026
> [!NOTE] > **TL;DR** > - enable multiple accounts > - no UX changes for now -> #1459 Stacked on #1450 (base `claude/secret-storage-v2-1420`), which is stacked on #1434. Retarget once those land. Closes #1386. Part of #1297. ## Change - `replaceStoredAccount()` → `upsertProfile()`. It keeps other profiles. A stored profile keeps its own `secretsBackend` and file secrets on re-login. `loggedInAt` is refreshed on every login. - Profile `name` = `username`, falling back to the user ID. - The file-level `secretsBackend` is set only on a new file. A new or re-logged profile whose backend differs records its own, so a login with `APIFY_DISABLE_KEYRING=1` cannot redirect other accounts' reads. - Login runs the migrations first, so a v1 account survives logging in to a second one. The v1 backup is kept, since that account is still stored. - Login no longer clears the outgoing account's keyed keyring entries. On an account switch it still clears the legacy unkeyed entries and top-level file secrets, so a later keying pass cannot file them under the new account. - `removeActiveProfile()` picks the next active profile by `loggedInAt` (`null` last) and returns the removed and new active profile. Telemetry user ID follows the new active one. - Messages: with one account, nothing changes. Login adds `Other stored accounts: A.` when others exist. Logout says `You are logged out of B. A is now the active account.` - Logout description updated; `docs/` regenerated. ## Verification - `pnpm run test:local` — 687 passed, 4 skipped (64 files). - `pnpm run lint`, `pnpm run format`, `pnpm run build` — clean. `pnpm run update-docs` — logout description only. - `pnpm run test:api` not run — no token in this environment. - New tests cover the issue's cases: two logins → two profiles, second active; re-login → updated in place and active; logout with two → other active with message; logout with one → logged out; v1 file then second login. Also: next active by `loggedInAt`, keyring entries kept per account, logout clearing only the outgoing account, and a keyring-disabled login leaving other accounts on the keyring. - Install size unchanged — no dependency added or removed. ## Left out - `--profile`, `auth switch`, `auth list`, `logout --profile` / `--all` — Stage-3 (#1384). - Profiles migrated from v1 keep `name: null` until their next login. Stage-3 may want the migration to set it. - `login -m console` becomes additive through `tryToLogin`, with no dedicated tests, as the issue says. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Note
TL;DR
profilesshape (userIDkeys)profilesstructure)tokenattribute will be migrate in followup with token in keyring (same for proxy password)apify loginagain — see the release note belowPart of #1383 (Stage-1). Closes #1419.
What changed
New
src/lib/auth-file.tsowns the file — reading, an atomic write, the migration, and the profile accessors.credentials.ts,login,logout,getLocalUserInfo()and the rental-sunset notice all go through it. No caller parsesauth.jsonby hand any more.{ "version": 2, "activeProfile": "<userId>", "profiles": { "<userId>": { "username": "moria", "name": null, ... } }, "secretsBackend": "keyring" }token/proxy.passwordon the file backend. Secret storage v2 #1420 moves both backends together, becausedowngradeBackendToFile()can flip mid-process and a half-moved scheme loses a token.name,authMethod,expiresAt,hasRefreshToken,loggedInAt, per-profilesecretsBackend) are written and never read, so the device flow and Secret storage v2 #1420 need no extra format change.loggedInAtis recorded from the start becauseauth listwill order by it and a logout will fall back to the most recent profile left — neither can backfill a time nobody wrote down.email,plan,effectivePlatformFeatures,isPaying,createdAt,proxy.groups. Checked all 19 call sites — no readers.emailalso leaves theAuthJSONtype.getLocalUserInfo()keeps its return shape, so its call sites are untouched.0600. That also repairs a legacy0644auth.json, which the old in-place write never did.Migration
A version-step chain, so moving data between shapes later means adding a table entry rather than reworking the module. Runs after
ensureMigrated(), is single-flight, and never throws — a failure leaves the old shape, which the readers still understand, and says so once.keyringfileidhas no key to store a profile under. The secrets stay and the next command asks for a re-login; no key is invented.apify logoutas the way out.Behavior worth calling out
apify logoutroutes through the store instead ofrimraf. It drops the active profile and its secrets, removes the backup, and never refuses — it is the only escape from a file this CLI cannot read, so a shape it does not understand is discarded whole rather than edited.The version guard runs on the stored-login path only. A platform run or CI job has
APIFY_TOKENas its only credential and no interest in the stored file; checking it first would stopapify runover a file it never reads.A downgrade needs a release note. An older CLI is not fully compatible with a v2 file, verified against 1.10.0:
apify infoand other commands that call the API first work — the per-command refresh re-adds the flatid/usernamethe old code reads.apify runthrowsStale credentials found without user metadataon a freshly migrated file. It reads the local file without that refresh, and a v2 file has no flatid/usernameuntil some other command puts them back.apify loginmixes two accounts. 1.10.0 merges into the existing file rather than replacing it, soactiveProfileandprofileskeep the old account while the flat fields and the token become the new one. This CLI then shows one account and authenticates as the other.Release note: after upgrading, run
apify loginagain before using an older CLI against the same machine.Verification
pnpm run test:local— 665 passed, 4 skipped, up 12 from master.pnpm run lint,pnpm run format,pnpm run build— clean.pnpm run update-docs— no diff; no flag, arg, description or registration moved.36a7004d, including API Tests and Windows on Node 22 and 24.apify runinjecting the right identity,actors lsagainst the API, the installed CLI reading the migrated file, and logout clearing both files.test/local/lib/auth-file.test.tscovers states A/B/C on both backends, idempotency, the backup contents and mode, a corrupt file, a v1 file with noid, a danglingactiveProfile, aversion: 3file surviving a login and discarded by a logout, and the migration's failure path.Left out, deliberately
--profile, noauth switch/list. The file can hold N profiles; Stage-2 (Stage-2: Login - token multi account support #1386) puts a second one there, Stage-3 (Stage-3: Multi-account UX #1384) adds the commands.🤖 Generated with Claude Code