Skip to content

Setting an owner token silently does nothing when the OS secret store is unavailable: the one-time refusal report was already spent at startup #477

Description

@matt-edmondson

What's wrong

ApplyOwnerToken (ProjectDirector/ProjectDirector.cs ~L338–341) returns TokenStorage.DrainUnavailableReport() when WriteOwnerToken refuses. Its own doc comment says a refusal "has to reach the user: the popup closes either way, so silence would look like success". But TokenStorage.ReportUnavailable (ProjectDirector/TokenStorage.cs ~L238) fills the report only on the first failure in the process, guarded by _unavailableReported.

At startup the constructor spends that report:

  1. PrepareTokens calls ResolveGitHubCredentials(..., options.GitHubToken).
  2. The getter reads the store and the read fails, which sets the report.
  3. DrainSecretStoreReport() logs it and clears it.

From then on, every "Set GitHub Owner Token" against the unavailable store gets false from WriteOwnerToken, and DrainUnavailableReport() returns "". Nothing is logged, the popup closes, and the token is discarded. A transient store failure, such as a locked keychain or libsecret throwing CredentialStoreException, goes silent the same way after its first occurrence.

The existing ApplyOwnerTokenReportsARefusal test passes only because UseCache resets the flag immediately before the call. It never exercises the startup → drain → apply sequence.

Failure scenario

  1. Run on Linux without a Secret Service provider.
  2. At launch the log shows the "No usable OS secret store..." line once.
  3. File → Set GitHub Owner Token → ktsu-dev → paste a PAT → OK.
  4. Nothing is logged, and the next scan still runs unauthenticated for that owner.

Verified with a scratch MSTest whose store throws DllNotFoundException:

  1. PrepareTokens(new options) returns a non-empty startup report.
  2. DrainUnavailableReport() is called, as the constructor does.
  3. ApplyOwnerToken("ktsu-dev", "ghp_typed") returns "".

Suggested fix / acceptance criteria

  • When WriteOwnerToken returns false, have ApplyOwnerToken always produce a message, not only when the once-per-process report is pending. For example: $"Could not save the token for {owner}: " + (TokenStorage.DrainUnavailableReport() is { Length: > 0 } r ? r : "the OS secret store is unavailable").
  • Alternatively, keep the last failure reason separately from the "already reported" flag.
  • Acceptance: a test that runs startup → drain → ApplyOwnerToken against an unavailable store gets a non-empty message, and that message is logged.

Activity

  1. matt-edmondson commented on Oct 6, 2026

    @matt-edmondson
    ContributorAuthor

    Triage

    • Category: Bug
    • Priority: Medium. A token entered by the user is discarded silently when the OS secret store is unavailable. This only affects environments without a usable store, such as Linux without Secret Service or a locked keychain.
    • Area / suggested assignment: Token storage and credentials (TokenStorage.cs, ApplyOwnerToken).
    • Duplicates / related: No duplicate found.
    • In progress: No open PR covers it.
    • Notes: The one-line fallback message in the issue is enough. Also tighten ApplyOwnerTokenReportsARefusal so it runs startup → drain → apply rather than resetting the flag first.

    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions