Repository navigation
Conversation
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>
l2ysho
force-pushed
the
claude/secret-storage-v2-1420
branch
from
September 23, 2026 21:31
35a698c to
510f799
Compare
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
force-pushed
the
claude/secret-storage-v2-1420
branch
from
September 24, 2026 12:38
a39ad9f to
95f8bfd
Compare
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>
Secrets lived under one fixed name per kind, so a second account would overwrite the first one's token. Both the keyring and the file backend are now keyed by user ID, and existing secrets are re-keyed in place. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A keyring write that fails for one account used to flip the file-level marker, sending every other account to a file that does not hold their secrets. The fallback is now recorded on the profile that hit it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
l2ysho
force-pushed
the
claude/secret-storage-v2-1420
branch
from
September 24, 2026 14:08
95f8bfd to
85ac3fd
Compare
auth.json is the only index of what the keyring holds, so both commands destroyed entries before a step that can throw. A failed account switch left the outgoing account's token deleted and the new one unwritten, and a failed logout destroyed the secrets while auth.json still named the account. Login now clears after the switch is on disk; logout attempts both steps and reports what is left behind instead of claiming success. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Secret reads are v2-only, so a failed shape migration leaves the CLI unable to read the token. It still told the user their login worked, and the next command said they were not logged in. The warning now states what is true and how to fix it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
auth.json is replaced through a temp file and a rename, so the write needs the directory to be writable and the file's own mode never matters. Telling the user to make the file writable sends them to change something that has no effect. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The keyring is used unless APIFY_DISABLE_KEYRING=1 is set or the module cannot load. A token stored in the profile is the only record that its secrets live in the file, so a past keyring failure no longer pins later logins to plaintext, and a successful keyring write clears the file copies. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`keyKeyringSecrets()` copied a legacy keyring entry onto the keyed name without checking what was already there. A login runs before the migration does, so an upgraded user who logged in first had the new token replaced by the old one on their next command. Skip the copy when the account already has a secret of that kind, and delete the legacy entry instead. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The three migrations run in a fixed order, because keying secrets by user needs the ID the shape migration produces. That order was typed out at both call sites and documented in a comment. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A login writes its secrets under keyed names, so anything left under the fixed names is stale. It survived a repeat login of the same account, and the next migration cannot tell it from a current secret: a proxy password the account had dropped came back. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`deleteKeyring()` swallowed every error, so `clearKeyringSecrets()` could not reject and logout's failure branch was unreachable. On a locked keyring, logout said the secrets were removed and exited 0, while deleting the only index of what the keyring holds. Every key is still attempted, and login warns rather than failing when it cannot remove the previous account's entries. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`getToken()` and `getProxyPassword()` are gone. Vitest resolved the missing imports to undefined instead of failing, so the suite died at the call. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`backendFor()` sends every read for an account to the file once its token is there, so a proxy password left in the keyring could not be read. A local run lost its proxy password until the next login. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The keyring module loads on machines where the secret service does not answer, so every entry throws although nothing was ever stored. Logout took that for a refused delete and exited 1, telling the user to clean a keyring they do not have, after removing their account cleanly. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A keyed token means a login already stored this account's secrets under the new names, so the fixed names hold whatever a previous login left. The per-kind guard could not see that: a login clears the proxy password it has none of, and the migration then read the previous account's under the new one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The check for whether the fixed names are this account's ran on every command. On a keyed account they hold nothing, so it never decided anything. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`clearKeyringSecrets` threw an AggregateError of bare messages, so neither caller knew which entry survived. Logout named the account it had just cleared successfully and never named the entry that was still there. Login said nothing at all when the account had not changed, leaving a live token in the keyring with nothing to ever mention it. It now returns the surviving entries. Nothing wanted an exception: both callers turned it straight back into a value. Logout also keeps its report of APIFY_TOKEN and clears the telemetry ID when the profile is gone, which the early return used to skip. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`deleteSecret()` is what stops a revoked proxy password surviving a re-login, and it was the one delete whose failure nothing reported. The stored value stays, and reads serve it before the file, so every later local run gets a password the account no longer has. It now returns what it left behind, and login folds that into the warning it already prints. Also corrects a claim repeated in four places: `@napi-rs/keyring` does export `findCredentials()`. The CLI choosing not to enumerate is the real reason auth.json is its only index. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A comment moved with login's warning and kept describing the narrower set it used to cover: `getSecret` reads the keyring before the file, so the proxy password it now reports is read on every command. `describeLeftovers` does not print what a keyring app shows — Windows and Linux Secret Service both name an entry differently. `ensureCredentialsCurrent` can throw, through `ensureAuthFileCurrent`. And `findCredentials()` throws on a keyutils machine rather than returning nothing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`__APIFY_INTERNAL_TEST_AUTH_PATH__` moves auth.json somewhere scratch, but the OS keyring is per-user and `clearKeyringSecrets()` deletes the fixed names whatever the backend is. Two of the seven test files that reach credentials never mocked the module, and one of them runs `logout`, so `test:api` deleted the developer's own stored login. Mocked for every test file instead of per file, with `useAuthSetup` refusing to run if the real module ever resolves again. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
l2ysho
marked this pull request as ready for review
October 2, 2026 07:08
…ge-v2-1420-f1 # Conflicts: # src/commands/auth/logout.ts # src/lib/auth-file.ts # src/lib/auth.ts # src/lib/credentials.ts # src/lib/utils.ts # test/__setup__/auth-file.ts # test/api/commands/log_in_out.test.ts # test/local/commands/auth.test.ts # test/local/commands/run.test.ts # test/local/lib/auth-file.test.ts # test/local/lib/auth.test.ts # test/local/lib/credentials.test.ts
This was
linked to
issues
Oct 5, 2026
This branch has not been deployed
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 —
Stacked on #1434 (Stage-1, subtask 4). Base branch is
claude/auth-json-v2-1419, notmaster.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:
logoutused to exit 0 and say the keyring was cleared even when it was not. It now exits 1 and names the entry left behind, byservice/account.loginwarns 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.One contract change worth a release note: on a machine where the keyring exists but refuses a delete,
logoutnow 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, noauth switch.Change
com.apify.cli/token,proxy-passwordcom.apify.cli.token,com.apify.cli.proxy-password/<userId>auth.json.token,auth.json.proxy.passwordprofiles[<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/deleteProxyPasswordcollapse intogetSecret(userId, kind),setSecret(userId, kind, value)anddeleteSecret(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 inauth.jsonmeans 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.loginWithToken()clears the fixed names on every login, including a repeat of the same account.A v1 file with a token but no
idhas 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.jsonorphans 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.APIFY_DISABLE_KEYRINGtoggled between login and logout.@napi-rs/keyringglobally.__APIFY_INTERNAL_TEST_AUTH_PATH__relocatesauth.jsonbut not the OS keyring, and two test files that runlogoutwere 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:
logoutexited 1, namedcom.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.MIGRATION_STEPSinauth-file.tsis(file) => file, synchronous and pure, so keying could not be a versioned step. Widening it and bumpingAUTH_FILE_VERSIONto 3 retires the guards, the probe, and the file-shape helpers still incredentials.ts.test/is not typechecked (tsconfig.jsonis"include": ["src"]), which is how thetest:apiimports broke unnoticed.Left out
--profile, noauth switch, noauth list.clearKeyringSecrets()takes the profile whose entries to clear, but only ever sees one profile today — Stage-2 (Stage-2: Login - token multi account support #1386) is what puts a second one there.🤖 Generated with Claude Code