[JSdotNet]

Closes #753

Summary

Every stored third-party credential now goes through the OS-protected envelope. A shared SecretEnvelope (DPAPI CurrentUser, value-level dpapi:<base64>) seals the GitHub account and repository tokens in github.json, the Claude AdminApiKey in claude.json and the Azure Foundry ApiKey in azure-foundry.json; every non-secret field stays readable JSON. Legacy plaintext is migrated on first read; an envelope that cannot be opened reads as "no key". IDeviceCredentialStore.Save becomes SaveAsync, so the Android pairing save no longer blocks. Recorded as proposed local ADR 0030 with the threat model (a bearer key that works from any machine vs. a file copied off it), keeping ADR 0012's loopback MCP token as the stated exception; R8, the crosscutting Authentication section and tech/desktop.md are reconciled.

Decision taken without asking (follow it, or ask for the alternative)

Question. How are GitHub, Claude and Azure Foundry credentials stored at rest: plaintext accepted under the user profile (Option A), or through the DPAPI / platform secure-store envelope (Option B)?

Chosen. Option B: every stored third-party credential goes through the OS-protected envelope (DPAPI CurrentUser on Windows; the three stores are desktop-only, so no Android work for them). Recorded as new local ADR 0030 (proposed). Applied as a value-level envelope on the four secret fields, so the JSON files stay legible. Legacy plaintext is migrated on first read, and an unopenable envelope reads as 'no key'. Also adds IDeviceCredentialStore.SaveAsync so the Android pairing save no longer blocks.

Alternative. Option A: state in an ADR 0012 amendment or new ADR that secrets under %LOCALAPPDATA% are protected by the OS profile and plaintext is accepted, with the threat model. Then add a doc comment to each of the three stores, as WorkspaceSettingsStore has for the MCP token.

Why the choice. Five pieces of repository evidence favour B. (1) Risk R8 in 11-risks-and-technical-debt.md already names 'use OS secure storage' as the mitigation for on-device external credentials. (2) The two credentials added most recently, the sync device credential and the spec-manager refresh token, both use DPAPI CurrentUser. DeviceCredentialStoreFactory and SpecManagerTokenStoreFactory both say a secret written in the clear is 'the one thing that would not be acceptable', and the spec-manager store chose in-memory over plaintext on non-Windows. (3) ADR 0012's plaintext argument is specific: the MCP token guards a loopback server for the same user, so encrypting it against that user defends nothing. A GitHub PAT, an Anthropic admin key or an Azure key is a bearer credential that works from any machine. DPAPI CurrentUser does defend against file copies (backups, disk images, diagnostic zips, other accounts, offline disk reads), and that asymmetry is the threat model. (4) The envelope helper already exists twice in the codebase, ProtectedData is centrally versioned, and CI tests run on windows-latest. (5) The value-level envelope keeps every structural test assertion and the hand-readable settings files intact, so the change stays confined to three stores and a handful of token-text assertions.

To follow the alternative instead. Following A instead would leave GitHubSettings.cs, ClaudeSettings.cs, AzureFoundrySettings.cs and the three csproj files unchanged apart from a doc comment on each store citing the ADR. It would drop the SecretEnvelope helper, the migration code and the flipped test assertions (RepositoryRegistrySplitTests would keep Assert.Contains("ghp_secret", local)). ADR 0030 would record A as chosen and B as the considered option, and R8 would be closed as accepted rather than mitigated. The IDeviceCredentialStore.SaveAsync part is independent of the choice and could stay either way.

Acceptance criteria

• 1. New local ADR 0030 with status: proposed, meta block, threat model, decision covering all five credential kinds, Option A as a considered option with the reason it lost, and the ADR 0012 MCP-token exception — not proved by a test: no test
• 2. ADR listed in adr/README.md and linked from 09; R8, 08 Authentication and Authorization, and the tech/desktop.md ProtectedData entry reconciled; 0029 not used — not proved by a test: no test
• 3. node .devbook/_tools/devbook-meta/build.mjs --check and node tools/devbook/check-metadata.mjs both pass — not proved by a test: no test
• 4. GitHub account PAT and repository token are not in github.json after save; a new store returns them — test: GitHubCredentialsAtRestTests.An_account_token_is_not_in_the_file_and_a_restart_reads_it_back; GitHubCredentialsAtRestTests.A_repository_token_is_not_in_the_file_and_a_restart_reads_it_back; RepositoryRegistrySplitTests.The_registry_holds_identity_and_the_local_file_holds_the_machine_half; AccountBindingTests.The_binding_is_registry_data_and_the_credential_is_local
• 5. Claude AdminApiKey is not in claude.json after SetAdminApiKey; a reopened store returns it — test: ClaudeSettingsStoreTests.A_saved_key_is_not_in_the_file_and_a_restart_reads_it_back
• 6. Azure Foundry ApiKey is not in azure-foundry.json after SetApiKey or SetConnection; a reopened store returns it — test: AzureFoundrySettingsStoreTests.A_saved_key_is_not_in_the_file_and_a_restart_reads_it_back
• 7. Non-secret fields stay plain, readable JSON — test: The at-rest tests above assert the id, login, cloneDirectory, apiEndpoint, endpoint, deployment, actor and costScope stay readable in the file; the existing structural assertions in RepositoryRegistrySplitTests and AccountBindingTests still pass
• 8. Migration on first read: plaintext loads into Current, the file then holds only the protected form; the legacy Claude top-level adminApiKey and the legacy GitHub global token migrate — test: GitHubCredentialsAtRestTests.Plaintext_tokens_from_an_earlier_release_are_read_and_sealed_on_first_read; GitHubCredentialsAtRestTests.The_legacy_global_token_still_reaches_the_repositories_and_is_not_left_in_the_clear; RepositoryRegistrySplitTests.A_legacy_github_json_carries_over_once_and_the_local_file_is_reduced; RepositoryRegistrySplitTests.A_corrupt_registry_never_prunes_the_overlay; ClaudeSettingsStoreTests.A_plaintext_key_from_an_earlier_release_is_read_and_sealed_on_first_read; ClaudeSettingsStoreTests.A_flat_settings_files_plaintext_key_is_sealed_on_first_read; AzureFoundrySettingsStoreTests.A_plaintext_key_from_an_earlier_release_is_read_and_sealed_on_first_read
• 9. A protected value this user cannot open reads as null without throwing, and the rest of the settings load — test: GitHubCredentialsAtRestTests.A_token_this_user_cannot_open_reads_as_no_token_and_the_rest_still_loads; ClaudeSettingsStoreTests.A_key_this_user_cannot_open_reads_as_no_key_and_the_rest_still_loads; AzureFoundrySettingsStoreTests.A_key_this_user_cannot_open_reads_as_no_key_and_the_rest_still_loads (each a Theory over 3 corrupt envelopes)
• 10. Tokens never reach repos.json or the replicated accounts document, and a protection-only change does not restamp the accounts document — test: GitHubCredentialsAtRestTests.Sealing_a_token_never_reaches_the_shared_documents_and_does_not_restamp_the_accounts. This is a regression guard: it also passed before the change, because nothing restamped then either.
• 11. IDeviceCredentialStore has SaveAsync; DevicePairingClient awaits it and keeps the CredentialStoreFailed mapping; SecureValueDeviceCredentialStore awaits SetAsync without GetAwaiter().GetResult(), and a failed write throws IOException with Current and Changed untouched — test: SecureValueDeviceCredentialStoreTests.Saving_awaits_the_platform_write_and_adopts_the_credential_only_after_it_lands; SecureValueDeviceCredentialStoreTests.A_credential_that_cannot_be_written_is_not_kept_and_nothing_says_it_changed (Theory: throw from the call, and fault through the returned task); DevicePairingClientTests.A_credential_whose_save_fails_after_it_was_handed_off_is_a_failed_pairing; DevicePairingClientTests.A_credential_that_cannot_be_stored_is_a_failed_pairing; DpapiDeviceCredentialStoreTests.Saving_through_the_port_writes_the_same_envelope
• 12. Each of the three settings stores has a doc comment naming ADR 0030 and stating what is protected and why — not proved by a test: no test
• 13. dotnet build Backlog.sln (including the Android and Windows heads) and the unit suite pass; the CI Release -warnaserror slnf build stays warning-free — not proved by a test: no test

Testing

• Command: cd "D:/iswp/753-credentials-at-rest" && dotnet build Backlog.sln ; dotnet build Backlog.WithoutAppHeads.slnf --configuration Release -p:TreatWarningsAsErrors=true --no-incremental ; dotnet test --solution Backlog.WithoutAppHeads.slnf --no-build --configuration Release --max-parallel-test-modules 1 --parallel none
• Passed 16271, failed 0; repair attempts 0.
• Owner session, after the review fixes: Both devbook checks pass (build.mjs --check; check-metadata.mjs: 0 blocking).

Review

• Left open, minor (src/Infrastructure/Backlog.Infrastructure.GitHub/SecretEnvelope.cs:93): Open turns any envelope it cannot decrypt into null, and every later write in all three stores turns that null into a deleted secret. That includes the new write-back on first read. The writes re-seal what is in memory (Seal(null) is null), and the GitHub store drops a row whose only machine-local data was the token (CarriesMachineLocalData). So an envelope that failed for a temporary reason (for example the DPAPI master key cannot be decrypted while a domain-joined PC is offline after a password change), or that belongs to another profile, is overwritten the next time anything is saved. The ADR only accepts reading it as no key; it does not say the value is then destroyed.
• Left open, minor (src/Infrastructure/Backlog.Infrastructure.AzureFoundry/AzureFoundrySettings.cs:57): The new Write and Read paths each copy every AzureFoundrySettings field by hand, just to swap ApiKey for its envelope. That makes two more copies of the field list that Normalize already holds, and it pushes ciphertext through the domain model rather than through a protected DTO field as the scope asked. ClaudeSettingsStore does the same swap with a single with expression.

What could not be proved

• One-way data migration on real user files. On first start after upgrade, %LOCALAPPDATA%\Backlog\github.json, claude.json and azure-foundry.json are rewritten with dpapi: values. An older build reading a migrated file would send the envelope text as the key (downgrade hazard), and copying the files to another machine or profile now yields empty keys — re-entering them in Settings is the recovery.
• Check by hand: (1) start the desktop app over a profile with GitHub PATs, a Claude admin key and a Foundry key — the three files hold dpapi: values, Settings still shows each key configured, and GitHub calls, Claude usage and Foundry chat still work; (2) in the desktop web harness, the obj/local-development/*.settings.json files migrate and SeedLocalAzureFoundrySettings still recognises its marker key; (3) on the Android emulator, pairing still succeeds and survives a restart through the awaited SecureStorage write.
• Public port change: IDeviceCredentialStore.Save is replaced by SaveAsync (the Android head implements it).
• Open minor worth a decision: an envelope that fails to open for a transient reason reads as null, and the next write of that store (including the migration write-back) then deletes the secret. For a profile or machine move that is correct — the key is unrecoverable anyway — but a transient DPAPI failure would also lose it.
• Off Windows (development harness only) the envelope passes the value through as plaintext; ADR 0030 records it.

Merge Danger

Door. One-way. The first start migrates the three credential files in place; a revert leaves dpapi: values an older build cannot read (re-enter the keys). Decision record: local ADR 0030, proposed.

Blast radius. consumers — every desktop install's stored GitHub, Claude and Azure Foundry credentials, and the Android head's pairing save.

Assumptions taken instead of asking

• Non-Windows hosts: assumed the envelope does not write the secret in the clear off Windows, and the secret is kept for the process lifetime only, as SpecManagerTokenStoreFactory and DeviceCredentialStoreFactory do. No shipped head runs these stores off Windows; only the desktop web harness could, and only in development. GitHubSettingsStore.Reload re-reads the file, so on non-Windows a secret entered that run would be lost on reload; accepted as dev-only.
• Envelope shape: assumed value-level (a prefixed base64 DPAPI blob in the existing JSON field, e.g. "dpapi:...") rather than whole-file like DpapiDeviceCredentialStore. That keeps non-secret settings readable and keeps the structural test assertions. Whole-file would match the existing two stores more closely but break dozens of assertions and hand-editing.
• Shared helper location: assumed one internal SecretEnvelope.cs linked into the three projects via , as IsoTimestamps.cs is, plus a ProtectedData PackageReference each. Assumed it stays out of Backlog.SharedKernel so Core takes no platform-crypto dependency.
• SaveAsync scope: assumed SaveAsync is added to IDeviceCredentialStore and implemented by all four implementors plus the test doubles, with DevicePairingClient switched to it. Whether the synchronous Save is removed or kept (with a justification comment) is left to the implementation stage; removing it is preferred if no other caller exists. Only DevicePairingClient and the Android adapter call it today.
• ADR form: assumed a new numbered local ADR 0030, following the repository's existing numbered-ADR practice and the run policy, rather than the devbook rule's one-record-per-concern slug files. ADR 0012 is not amended beyond an optional cross-reference.
• Rotation/backup story for R8: assumed the answer is that protected keys do not survive a profile or machine move. Recovery is re-entering the key in Settings, ADR 0010 backups contain no credential files, and rotation stays at each provider.
• I did not invoke the delivery:flow-code / delivery:flow-spec skills. I treated this sub-agent as the implement stage of the issue-sweep workflow that already owns the gate, Personal Validation and the PR.
• The envelope is a value-level dpapi: + base64 prefix. A value without the prefix is treated as legacy plaintext. No real GitHub, Anthropic or Azure key starts with dpapi:.
• On a platform without DPAPI (only a development harness off Windows), SecretEnvelope passes the value through as plaintext rather than refusing to store it. ADR 0030 records this as a neutral consequence.
• IDeviceCredentialStore.Save was replaced by SaveAsync rather than having SaveAsync added beside it. Keeping Save on the port would force SecureValueDeviceCredentialStore to keep a blocking save. DpapiDeviceCredentialStore and InMemoryDeviceCredentialStore keep a public synchronous Save for existing tests and callers of the concrete types.
• DevicePairingClient passes CancellationToken.None to SaveAsync: once the service has issued a credential, cancelling the save would lose the only copy.
• The GitHub legacy global token keeps its existing semantics. It is carried onto each repository row and written back as null, so a file with a global token and no repositories still loses it on the first write, as any save already did.
• The GitHub migration write on first read is skipped when a registry carry-over was attempted and failed, so the legacy identity fields are not dropped. The next start seals the tokens.
• ADR 0030's verification and decision sections describe the device-credential and spec-manager stores as already DPAPI-sealed. No code change was made to the spec-manager store.
• The Backlog.Desktop.UI.UnitTests failures in the full run are timing failures under concurrent load (devbook pane, brain conflict, calendar hours; mostly canceled). They differ between runs, pass when their classes run alone, and touch no credential code.

---

Opened as a draft by an attended run of delivery-schedule:schedule-issue-sweep (2026-10-15), told by the owner to decide without asking and to name the choice and the alternative.

🤖 Generated with Claude Code