Delegate provider token storage to ktsu.CredentialCache

[matt-edmondson]

What's hand-rolled

BuildProvider and Owner each hold a personal access token as a plain [JsonInclude]-annotated BuildProviderToken (a SemanticString) field:

• BuildProvider.Token —

  BuildMonitor/BuildMonitor/BuildProvider.cs

  Lines 49 to 51 in 0b84482

  	[JsonInclude]
  	protected BuildProviderToken Token { get; private set; } = new();
  	[JsonInclude]

• Owner.Token (per-owner override) —

  BuildMonitor/BuildMonitor/Owner.cs

  Lines 21 to 26 in 0b84482

  	/// <summary>
  	/// Optional token for this specific owner. If set, overrides the provider-level token.
  	/// Useful for accessing private repositories in different organizations.
  	/// </summary>
  	[JsonInclude]
  	public BuildProviderToken Token { get; internal set; } = new();

Both BuildProvider and Owner hang off AppData.BuildProviders (

BuildMonitor/BuildMonitor/AppData.cs

Line 15 in 0b84482

public ConcurrentDictionary<BuildProviderName, BuildProvider> BuildProviders { get; set; } = [];

), which ktsu.AppDataStorage serializes as a whole and writes to a JSON file under the user's AppData directory on every save (BuildMonitor.QueueSaveAppData() is called right after the token is set — e.g.

BuildMonitor/BuildMonitor/BuildProvider.cs

Lines 212 to 216 in 0b84482

	ShouldShowAccountIdPopup = false;
	}
	else if (ShouldShowTokenPopup)
	{
	PopupInputString.Open(Strings.SetToken, Strings.Token, string.Empty, (result) =>

). The net effect: GitHub/Azure DevOps PATs (typically scoped to repo/workflow) end up sitting in plaintext JSON on disk rather than in an OS-native secret store.

What ktsu.CredentialCache provides

ktsu.CredentialCache.CredentialCache is an in-memory cache backed by a pluggable ICredentialStore, whose default (CredentialStoreFactory.CreateDefault()) routes to Windows Credential Manager, macOS Keychain, or Linux Secret Service (libsecret):

• CredentialCache.Instance.AddOrReplace(PersonaGUID persona, Credential credential) — persists before caching in memory (https://github.com/ktsu-dev/CredentialCache/blob/6286ebe59ae30768647f01fa50e22b579831a37e/CredentialCache/CredentialCache.cs#L140-L149)
• CredentialCache.Instance.TryGet(PersonaGUID persona, out Credential? credential) — loads from the store on cache miss (https://github.com/ktsu-dev/CredentialCache/blob/6286ebe59ae30768647f01fa50e22b579831a37e/CredentialCache/CredentialCache.cs#L120-L136)
• CredentialWithToken.Token (CredentialToken, a SemanticString) — a ready-made typed token credential (https://github.com/ktsu-dev/CredentialCache/blob/6286ebe59ae30768647f01fa50e22b579831a37e/CredentialCache/CredentialWithToken.cs)

Why it's worth it

This isn't a line-count win — it's a genuine gap between what the app currently does (plaintext-on-disk PAT storage) and what a purpose-built OS-backed secret store gets right (encryption at rest, OS-level access control, no secret sitting in a file a backup tool or another local process can read straight off disk).

Compatibility

• Subject (BuildMonitor) targets: net10.0
• ktsu.CredentialCache targets: net9.0;net10.0 — compatible
• Dependency direction: ktsu.CredentialCache's Directory.Packages.props references only ktsu.RoundTripStringJsonConverter, ktsu.Semantics.Strings, Polyfill, and System.Text.Json — no dependency on BuildMonitor, so no cycle

Sketch

Before (per provider/owner):

[JsonInclude]
protected BuildProviderToken Token { get; private set; } = new();

After:

PersonaGUID persona = /* derived/stored per provider or owner */;
CredentialCache.Instance.AddOrReplace(persona, new CredentialWithToken { Token = tokenValue.As<CredentialToken>() });
// ...
if (CredentialCache.Instance.TryGet(persona, out Credential? cred) && cred is CredentialWithToken t)
{
    // use t.Token
}

AppData would then store only the PersonaGUID (or nothing at all, if the persona ID is derived deterministically from BuildProviderName/OwnerName), not the token itself.

Caveats

• This changes the shape of BuildProvider/Owner's persisted state: existing AppData.json files with a Token field would need a one-time migration (read the old field once, write it into CredentialCache, then stop persisting it directly) rather than a silent swap.
• CredentialStoreFactory.CreateDefault()'s Linux backend depends on libsecret (Secret Service) being available; headless/container environments without a Secret Service provider would need a fallback story, which is only relevant if BuildMonitor is ever run outside a desktop session.
• This is a [JsonInclude]/internal implementation detail, not part of the module's tests or any public contract, so no public API changes for consumers of this repo.

[matt-edmondson]

Triage

Category: Improvement (security hardening)
Priority: High
Area: BuildMonitor — BuildProvider / Owner token persistence

Filed under the ktsu-reuse audit, and the issue body already makes the right argument: this is not a line-count win. GitHub and Azure DevOps PATs — typically repo/workflow-scoped — are serialized into plaintext JSON under the user's AppData directory on every save, readable by anything running as the same user. The priority reflects the exposure, not the consolidation.

This is one of three. The identical pattern is filed as ktsu-dev/OAICLI#42 (OpenAI API key) and ktsu-dev/ProjectDirector#411 (per-owner GitHub PATs). Not duplicates — three separate codebases — but one root cause (ktsu.AppDataStorage's general-purpose JSON persistence being used for secrets), one remedy, and one set of shared decisions worth settling once.

Suggested assignment: whoever owns BuildMonitor's provider/auth path, coordinating with the OAICLI and ProjectDirector owners.

Open PR covering this: none.

Suggested next steps:

• BuildMonitor has the most involved shape of the three — a two-level BuildProvider.Token plus per-Owner override. Settle persona derivation here and in ProjectDirector#411 together; OAICLI's single fixed persona is the trivial case and shouldn't drive the design.
• Prefer deterministic derivation from BuildProviderName/OwnerName over storing a PersonaGUID, if it holds up: it keeps AppData free of any credential-shaped state at all, and the override semantics (owner token shadowing provider token) stay expressible as a lookup order rather than extra persisted fields.
• Take the one-time migration the caveats describe, and additionally clear the old Token field from the existing AppData.json on upgrade. Ceasing to write a secret does not remove the one already on disk.
• The libsecret caveat is worth an explicit decision rather than leaving it to discovery — note that OAICLI#42 hits this harder, since a CLI genuinely runs headless; agreeing one fallback story across the cluster is cheaper than three.

---

Generated by Claude Code