feat: keep provider and owner access tokens in the OS secret store [minor]

[matt-edmondson]

Fixes #278.

The exposure

BuildProvider.Token and Owner.Token were plain [JsonInclude] fields. Both hang off AppData.BuildProviders, which ktsu.AppDataStorage serializes whole and writes to an unencrypted file on every QueueSaveAppData() — and that is called immediately after a token is entered. GitHub and Azure DevOps PATs, usually scoped to repo/workflow, were sitting in plaintext JSON under the user's app data directory.

They now go to the platform-native secret store through ktsu.CredentialCache, under a service name scoped to ktsu.BuildMonitor so they cannot collide with another ktsu tool's credentials on a shared host.

Design

Personas are derived, not stored. The triage preferred this over persisting a PersonaGUID, and it holds up: a versioned namespace plus the provider name, and the owner name as well for an override, hashed to a stable GUID. AppData is left with no credential-shaped state at all, and the owner-shadows-provider semantics stay a lookup order rather than extra persisted fields. The namespace carries a /v1/ because changing the derivation would orphan every token already in the store — a future change has to be a deliberate, visible one.

Scoping the owner persona by provider matters: the same organization name can exist on both GitHub and Azure DevOps, and OwnerPersonaIsNotTheProviderPersona pins that an override can never land on the provider's own entry.

Migration. LegacyToken keeps the old Token JSON name so OnStart can find what earlier versions wrote. Order is load-bearing: the token is written to the secret store first, so a store that throws cannot lose it, and only then blanked in the file. If the store refuses the write, the old copy is deliberately left alone — clearing it would destroy the only copy the user has. A token already in the store wins over a stale legacy one, but the stale one is still cleared, because ceasing to write a secret does not remove the one already on disk.

No secret store. This is where BuildMonitor departs from its sibling ktsu-dev/OAICLI#42, deliberately. OAICLI is a CLI and throws with an explanation. BuildMonitor is a desktop app whose token reads sit on request paths running every few seconds — an exception out of a read would take down the render loop, and throwing once per read would bury the log. So a missing store reads as "no token", is logged once per process with the cause and the remedy, and surfaces through the existing AuthFailed provider status. What it never does is fall back to a plain file.

Token keeps its original accessibility (protected get, private set), so no call site outside BuildProvider gained any reach it did not have.

Tests

BuildMonitor.Test/TokenStorageTests.cs, 14 new tests on top of the existing 34, driven through a BuildProvider stub with no network behaviour and an InMemoryCredentialStore. The no-store path uses a store whose every operation throws DllNotFoundException, which is what a missing libsecret-1.so.0 looks like.

Proven to fail without the fix, by three mutations on an otherwise unchanged tree:

Mutation	Result
Migration stops clearing the plaintext copy	3 failures: MigrationBlanksThePlaintextCopies, MigrationIsIdempotent, MigrationKeepsTheStoredTokenAndStillClearsTheStaleOne
Migration clears the plaintext copy even when the store refused the write	1 failure: MigrationKeepsTheLegacyTokenWhenTheStoreRefuses
The active token is persisted to app data again	1 failure: SerializedProviderCarriesNoToken

One note on that last test, because it was wrong first time and the mutation is what caught it. SemanticString implements IEnumerable<char>, so under bare JsonSerializerOptions a token serializes as {"$values":["g","h","p",…]} and a substring assertion for the token value never matches — the test passed for the wrong reason. It now builds its options the way AppDataStorage does, with RoundTripStringJsonConverterFactory registered, so the check is against the representation actually written to disk. It also asserts the legacy "Token" field serializes empty, so the migration's own target is pinned too.

All 48 pass with the implementation in place, and the solution builds clean with no warnings.

Docs

CLAUDE.md's Authentication and Credentials section now records where tokens live, the persona derivation and why it is versioned, the migration, and the no-store decision. README's GitHub setup section notes the secret store and that an existing token is moved automatically.

Note for the rest of the cluster

ktsu-dev/ProjectDirector#411 is the remaining one, and it is the same multi-credential shape as this. The derived-persona approach here should port directly — a namespace plus whatever names identify the credential — as should the write-then-clear migration order and the data-loss guard when the store refuses.

🤖 Generated with Claude Code

https://claude.ai/code/session_018AnVpPWKjVtXnAvd4bnzUL

---

Generated by Claude Code

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
89.3% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[matt-edmondson]

CI status: github-advanced-security is not this PR's failure

The "Code scanning AI findings" agent failed before analysing anything, on a billing quota:

_t [SessionModelError]: You have exceeded your monthly quota
  errorType: 'quota',
  statusCode: 402,

That is an account-level Copilot quota, not a finding about this diff. It failed identically, within the same few minutes, on two unrelated PRs in other repositories — ktsu-dev/OAICLI#44 and ktsu-dev/ProjectDirector#424 — which is the reproduction: those three share nothing but the account. The other scanners on this PR are green (Code Quality / CodeQL, and the code-scanning CodeQL run).

There is no fix to port into this PR: a 402 clears when the quota resets or is raised, and nothing in a diff changes it. I also cannot re-run it to confirm — the dynamic workflow is not retryable, and GitHub answers 403 This workflow run cannot be retried.

Flagging it so the red mark is not mistaken for a security finding against a change that is specifically about moving secrets out of a plaintext file. I'll keep watching this PR until it is green and mergeable, and will act on anything that is actually this PR's.

---

Generated by Claude Code