Skip to content

Rejected GLM Coding Plan key can remain after a failed plaintext replacement #24963

Description

@parkavenue9639

Problem

When OS encryption is unavailable, replacing a saved GLM Coding Plan key publishes the new plaintext file before Orca knows whether that file could be restricted to the current user. If restoring the previous key then fails before the restore is published, the rejected new key remains on disk even though the save reports failure.

On Windows the requested file mode is ignored, so the leftover file can keep the parent directory's inherited permissions.

Proposed fix

After a failed restore, delete the credential file only when it still contains the rejected replacement. A restore that already published the previous key must not be deleted.

Activity

  1. AmethystLiang commented on Oct 4, 2026

    @AmethystLiang
    Contributor

    Thanks for reporting. Looking into it

  2. parkavenue9639 commented on Oct 5, 2026

    @parkavenue9639
    ContributorAuthor

    Thanks for looking into this, @AmethystLiang!

    For whoever picks it up — @nwparker, I saw you folded this race into #24674 via 414439a, which keeps a co-author trailer for me — one small ask, and please feel free to say no. This would be my first merged commit as primary author in Orca (my earlier #23804 also landed via co-author on #24618), and I noticed the repo has gone out of its way before to keep credit with the original community PR (#14485 reverted a maintainer fix so #14453 could land). #24964 is small, applies cleanly to current main, and its checks pass — if it's practical to merge it directly and rebase the corresponding piece of #24674 on top, I'd be very grateful. Happy to do any of that work myself. If the stack makes it awkward, the co-author credit is already more than fair.

  3. nwparker commented on Oct 10, 2026

    @nwparker
    Contributor

    The report is valid and I reviewed the original fix. I updated #24964 in its original fork branch, retaining @parkavenue9639’s primary-author commits, and added the stronger competing-write protection previously prepared in #24674. The credential branch now includes that original history and uses the same implementation.

    The original failure cleanup and the additional staging races pass 84 tests across six focused store/writer suites. The exact reviewed head is 1c242de7c3743d915c94bd326dde7a66db578d57; typecheck and changed-code quality also pass. Hosted fork checks have been approved and are running.

    Recommend merging #24964 first once its checks pass, then #24674. Keep this issue open until the safety fix lands. The comparison/rename gap remains non-atomic and is documented in both PRs.

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

Metadata

Metadata

Assignees

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