Skip to content

fix: enforce session auth hostname invariant - #1059

Merged
EhabY merged 4 commits into
mainfrom
fix/session-auth-hostname-invariant
Aug 4, 2026
Merged

fix: enforce session auth hostname invariant#1059
EhabY merged 4 commits into
mainfrom
fix/session-auth-hostname-invariant

Conversation

@EhabY

@EhabY EhabY commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • enforce that session auth URLs match their secret-storage hostname on reads and writes
  • treat malformed or mismatched stored auth as a missing session with sanitized warnings
  • catch invalid file-based auth migration at its recovery boundary and continue to login

Addresses #1057.

Validation

  • pnpm test — 2,264 passed, 1 skipped
  • pnpm typecheck
  • NODE_OPTIONS=--max-old-space-size=4096 pnpm lint
  • pnpm format:check
Implementation plan
  1. Validate the hostname invariant centrally in SecretsManager.
  2. Let writes throw; degrade invalid reads to a missing session.
  3. Catch migration failures at the file-migration recovery boundary.
  4. Cover read/write enforcement, OAuth exact-URL validation, and migration fallback.

This pull request was generated by Coder Agents.

@EhabY EhabY self-assigned this Aug 3, 2026

@code-asher code-asher left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good but how does this fix #1057? Supposedly we are replacing coder.example.com with cursor.example.com but I see nothing in this diff that should affect that, just new assertions.

Unless it is related to the file migration somehow? But it seems like that would imply we somehow mangled the domain to begin with so the root bug must still be around somewhere.

Comment thread src/core/secretsManager.ts Outdated
Comment thread src/remote/remote.ts Outdated
Comment thread src/core/secretsManager.ts Outdated
@EhabY

EhabY commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator Author

The theory here is that this was because of a failed migration but this does not really fix it because there is nothing in our codebase that could do this, I have a suspicion that this is related to global settings not being applied to remote connection which is already fixed on main.

Also this would catch the users error much earlier and show more info, so hopefully we can understand how that happened.

Include the offending URL in the invalid-URL error and log the error
itself at both call sites instead of a generic message. The mismatch
branch still reports only hostnames, so a stored URL carrying
credentials does not reach the log.
@EhabY
EhabY merged commit 2552dab into main Aug 4, 2026
10 of 11 checks passed
@EhabY
EhabY deleted the fix/session-auth-hostname-invariant branch August 4, 2026 09:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants