Skip to content

fix(oauth): reject read-only credentials before starting login - #905

Merged
okwasniewski merged 2 commits into
tester-army:mainfrom
rudycelekli:fix/oauth-reject-readonly-store-before-login-20261005
Oct 7, 2026
Merged

okwasniewski merged 2 commits into
tester-army:mainfrom
rudycelekli:fix/oauth-reject-readonly-store-before-login-20261005

Conversation

@rudycelekli

@rudycelekli rudycelekli commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor
  • Reject the built-in read-only environment credential store before starting provider authentication.
  • Previously login completed the provider flow, then discovered it could not save the result. A supplied writable store still takes precedence over the environment variable.
  • Reuse the same store error for login, set and remove; retain logout's absent-login behavior. Add docs, consumer guidance and a changeset.

Verified

Ran it locally: yes

Invoked the actual built CLI in a temporary project with E2E_OAUTH_CREDENTIALS={} and e2e login github-copilot --client-id local-proof. Only that process's provider fetch was routed to a controlled loopback HTTP endpoint. No external credentials, browser authorization, model or provider account was used.

main branch
Sends a device-code request despite the read-only store; the held endpoint requires bounded process cleanup Sends no HTTP request and exits1 with the existing MISCONFIGURED explanation
Two new preflight regressions fail Default/explicit read-only stores refuse before provider login; the writable-store control passes
E2E_OAUTH_CREDENTIALS is set, so logins come from the environment and cannot be changed here; unset it to sign in on this machine
  • Full core unit suite:2453 passed, one skipped. Core typecheck and all seven package builds pass.
  • Full built CLI testbed:71 passed, one skipped. Its temporary4295 port change was restored before commit.
  • Lint, docs build validation and broken links pass. Separate fresh-context review found no critical or important findings.
  • pnpm check stops at the filtered install's unused Expo mobile dependency finding, also observed on main. Full native mobile verification requires those omitted dependencies. No runner, engine or model behavior was changed.
  • Fork Actions require upstream maintainer authorization. Hosted CI is not claimed green.

AI assistance: implemented and independently reviewed with Codex, with built CLI before/after evidence.

Current-head workflows awaiting maintainer authorization: docs, mobile, benchmark, spec.

Updated verification (October 6)

Moved afterEach into the login describe, kept only the requested unset-before-login sentence, and removed the skills addition. Focused tests and the rebuilt CLI readonly-store consumer pass.

Direct package typecheck and lint pass. The full monorepo check remains blocked at the same missing filtered mobile-benchmark dependency on untouched main; the current full Next benchmark was not rerun with this lean dependency environment. Hosted fork workflow authorization is reported separately; I am not marking these Ready for Human Review.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No issues found across 6 files

Re-trigger cubic

@okwasniewski

Copy link
Copy Markdown
Member

Makes sense, failing before the flow beats failing after it. Small asks: move the afterEach into the login describe, keep only "unset it before e2e login" in subscriptions.mdx, and drop the skills/setup.md change.

rudycelekli and others added 2 commits October 6, 2026 09:02
Signed-off-by: Rudy Celekli <47457359+rudycelekli@users.noreply.github.com>
Signed-off-by: Rudy Celekli <rudy@gradiahq.com>
@rudycelekli
rudycelekli force-pushed the fix/oauth-reject-readonly-store-before-login-20261005 branch from 5fc8787 to 1325f53 Compare October 6, 2026 13:28
@rudycelekli

Copy link
Copy Markdown
Contributor Author

[gpt-6.1-sol] responding on behalf of rudycelekli

Moved afterEach into the login describe, kept only the requested unset-before-login sentence, and removed the skills addition. Focused tests and the rebuilt CLI readonly-store consumer pass.

Direct package typecheck and lint pass. The full monorepo check remains blocked at the same missing filtered mobile-benchmark dependency on untouched main; the current full Next benchmark was not rerun with this lean dependency environment. Hosted fork workflow authorization is reported separately; I am not marking these Ready for Human Review.

@okwasniewski
okwasniewski merged commit ea0681a into tester-army:main Oct 7, 2026
19 checks passed
@github-actions github-actions Bot mentioned this pull request Oct 7, 2026
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