Twigpine/zero
1
0 Fork 0
mirror of https://github.com/Twigpine/zero.git synced 2026-11-09 18:08:31 -10:00
Code Issues Packages Projects Releases 1 Wiki Activity
728 Commits 235 Branches 13 Tags
fix/opencode-session-header
Commit Graph
3 Commits
Author SHA1 Message Date
Pujitha PaladuguandPujitha Paladugu 176f5a242b fix(oauth): force-close loopback callback connections past the shutdown deadline (#1051) 
* fix(oauth): force-close loopback callback connections past the shutdown deadline

Motivation:
LoopbackListener.Close shuts the callback server down with a 1-second
context and discards the result. http.Server.Shutdown only closes idle
connections, so a client that sends a partial request header and never
finishes it counts as active; Shutdown blocks out the full deadline,
times out, and Close returns while that connection and its Serve
goroutine remain open indefinitely. This is a loopback-only (127.0.0.1),
short-lived, single-use server, so the impact is a resource leak past
the end of the flow, not a remote vulnerability or crash.

Approach:
- add a ReadHeaderTimeout to the server so a stalled request header is
  bounded on its own;
- follow a timed-out or otherwise failed Shutdown with server.Close()
  to forcibly tear down any connection still open past the deadline;
- join the Serve goroutine (via a done channel) before Close returns,
  so its completion is observable instead of left dangling.

internal/mcp/oauth.go and internal/provideroauth/openrouter.go build
their own loopback servers with the same pattern; they are left
untouched here to keep this change scoped to the file the issue's
reproduction demonstrates.

Validation:
- go build ./internal/oauth/... and go vet ./internal/oauth/...: clean.
- gofmt -l internal/oauth/: clean (make fmt-check clean repo-wide).
- go test ./internal/oauth/... -race -v -count=1: all pass, including
  three new tests (TestLoopbackCloseForceClosesSlowHeaderConnection,
  TestLoopbackCloseIsIdempotent,
  TestLoopbackCloseReturnsPromptlyAfterSuccessfulCallback).
- Confirmed TestLoopbackCloseForceClosesSlowHeaderConnection fails
  against the pre-fix loopback.go (with the new test kept) and passes
  with the fix applied.
- Full repo go build ./..., go vet ./..., and go test ./... pass.
- Could not run make lint-static (golangci-lint) locally: it needs a
  very large dependency download and the local environment ran out of
  disk space fetching it. That check is continue-on-error (advisory)
  in .github/workflows/ci.yml, not a required gate.

Report: https://github.com/Gitlawb/zero/issues/1028
Signed-off-by: Pujitha Paladugu <[email protected]>
Assisted-by: claude-sonnet-5 (via Claude Code)

* test(oauth): synchronize loopback regression tests instead of sleeping

TestLoopbackCloseForceClosesSlowHeaderConnection relied on a fixed
100ms sleep to let Serve accept a connection before sending a stalled
header; under load this could fail or silently skip the force-close
path it's meant to exercise. Use an http.Server.ConnState hook (via a
new unexported newLoopbackListener, set before Serve starts to avoid
a race) to wait for the real StateNew signal instead.

TestLoopbackCloseReturnsPromptlyAfterSuccessfulCallback measured
Close's elapsed time without joining its http.Get goroutine or
draining/closing the response body; join it first so the timing
starts from a connection that's actually back to idle.

Also register t.Cleanup(l.Close) in both tests so a Fatalf between
construction and the existing explicit Close doesn't leak the
listener's Serve goroutine.

Signed-off-by: Pujitha Paladugu <[email protected]>

---------

Signed-off-by: Pujitha Paladugu <[email protected]>
Co-authored-by: Pujitha Paladugu <[email protected]>
 2026-09-15 08:37:58 +08:00
Vasanth TandClaude Opus 4.8 72a4229c2a fix(oauth): make ChatGPT (Codex) login work end-to-end (#264) 
* fix(oauth): add missing api.connectors scopes for ChatGPT OAuth preset

* fix(oauth): add missing authorize params for ChatGPT OAuth (id_token_add_organizations, codex_cli_simplified_flow, originator)

* fix(oauth): use localhost + /auth/callback for ChatGPT redirect URI (matches Codex CLI registration)

* fix(oauth): use fixed port 1455 + localhost/auth/callback for ChatGPT (matches opencode + Codex CLI client registration)

* chore: gofmt chatgpt.go

* refactor(oauth): use RedirectURIWithHost for ChatGPT callback; drop dead Port()

RedirectURIWithHost and Port() were added but never called — chatgpt.go
hardcoded "http://localhost:1455/auth/callback" as a literal, duplicating both
the port and path. Use RedirectURIWithHost("localhost", "/auth/callback") so the
redirect_uri derives its port from the listener (single source of truth via the
new chatgptCallbackPort constant), and remove the unused Port() method.

Also make the fixed-port bind failure actionable: ChatGPT's client registration
requires exactly port 1455, so we can't fall back to a random port — but if 1455
is already bound (a prior Zero/Codex login still open) the error now says so
instead of surfacing a raw OS bind error.

Tests: RedirectURIWithHost host/path formatting, and that the listener captures
the code on the /auth/callback path (not just /callback) — both were previously
uncovered.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>

* fix(oauth): restore api.connectors scopes for ChatGPT preset (review fix)

The fixed-port commit accidentally reverted internal/oauth/presets.go back to
only openid/profile/email/offline_access, dropping the api.connectors.read and
api.connectors.invoke scopes this PR exists to add — so authorize URLs still
omitted them and the authorize_hydra_invalid_request failure was not actually
fixed (caught in review by anandh8x).

Restore both connector scopes in the chatgpt preset and pin them with tests:
- TestResolveConfigChatGPTPreset now asserts both connector scopes are present
  (not just allowed).
- New TestChatGPTAuthorizeURLIncludesConnectorScopes inspects the generated
  authorize URL and asserts the scope param carries api.connectors.read and
  api.connectors.invoke, so a future revert is caught end-to-end.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>

* fix(oauth): read chatgpt_account_id from the nested OpenAI auth claim (P0)

extractChatGPTAccountID read chatgpt_account_id as a TOP-LEVEL id_token claim,
but auth.openai.com namespaces it under the "https://api.openai.com/auth" claim
object (the Codex CLI reads only from there). Against a real token the top-level
lookup returned "", so token.Account stayed empty → the codex provider omitted
the required chatgpt-account-id header on every request → Cloudflare/backend 401.

The login itself succeeded (bearer minted), so the failure was silent: the user
authenticated fine, then every Codex API call 401'd. The unit tests passed only
because the fixture put the claim at the top level, masking the bug.

Read the nested path first (id_token[ns][chatgpt_account_id]), matching codex;
keep a bare top-level claim as a forward-compat fallback only. Tests now build
the fixture with the nested shape a real token uses, plus regressions: a
top-level decoy must not shadow the nested value, and the top-level fallback
still works when the namespace is absent.

Found by an adversarial audit against the codex reference (same OAuth client).

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <[email protected]>
 2026-06-20 23:00:01 +05:30
KRATOS bb7de29c80 feat(oauth): reusable provider OAuth engine + zero auth CLI (#210) 
* feat(oauth): reusable provider OAuth engine + `zero auth` CLI

Add internal/oauth: a transport/identity-agnostic OAuth 2.0 engine that powers
provider login alongside the MCP-server OAuth zero already has. It is additive
and provider-neutral — NO providers, endpoints, or client identities are baked
into the binary; every provider is configured entirely via ZERO_OAUTH_<NAME>_*
env vars, so the engine works with any OAuth 2.0 / OIDC server.

Engine (internal/oauth):
- PKCE (S256 mandatory; "plain" refused) + per-flow CSRF state.
- Authorization-code flow: BuildAuthorizationURL / ExchangeCode / Refresh, with
  an https-only token-endpoint guard (loopback exempt) and error bodies that
  carry only error/error_description (never token material).
- Loopback callback server: 127.0.0.1-only, OS-assigned port, single use, state
  verified, then closed.
- Device-authorization grant (RFC 8628): RequestDeviceCode + PollDeviceToken
  honoring authorization_pending / slow_down / interval / expiry (headless/SSH).
- RFC 8414 + OIDC discovery (issuer-driven endpoint resolution).
- Namespaced token store ("provider:<name>" / "mcp:<name>"), 0600, file-locked
  (ownership-aware), atomic writes, malformed-fails-closed; GetFresh (on-demand
  refresh) + Handle401 (forced refresh).
- Opt-in proactive RefreshScheduler.

CLI (zero auth):
- login <provider> [--device] [--scope ...], logout, status [provider],
  refresh <provider> [--watch]. Status/output never print token material.
- Registered in app.go dispatch + top-level help.

MCP OAuth (internal/mcp) is left byte-for-byte unchanged (zero regression);
sharing the engine with MCP and encrypted-at-rest/keyring storage are documented
follow-ups. Tests cover every path (PKCE, auth URL, https-guard + redaction,
exchange/refresh, loopback capture + state-mismatch + timeout, device pending/
slow_down/expired, store namespacing/0600/locking, provider env resolution,
manager login (loopback+device)/GetFresh/Handle401, scheduler). Gates: gofmt /
vet / build (host+linux+windows) / test -race / staticcheck (no new) /
govulncheck (0) / deadcode (no new) all pass.

* oauth: fix Windows smoke — use OS-appropriate path in store-path test

TestResolveStorePathHonorsOverride hardcoded a unix "/tmp/custom/tok.json"
literal, which is not absolute on Windows (no drive), so ResolveStorePath
correctly resolves it against the current drive and the verbatim comparison
failed (D:\tmp\custom\tok.json). The production code is correct; the test now
builds the override with filepath.Join(t.TempDir(), ...) so it is absolute and
clean on every OS.

* oauth: address CodeRabbit review (11 findings)

Security / correctness:
- flow: caller-supplied ExtraAuthParams/extraParams can no longer override the
  reserved OAuth/PKCE fields (state, redirect_uri, code_challenge[_method],
  response_type, client_id) — e.g. forcing method=plain.
- device: a device-authorization response without expires_in now gets a bounded
  default lifetime (fail closed), and the poll loop re-checks expiry AFTER the
  interval sleep so it never polls past the deadline (e.g. after slow_down).
- lock: handle WriteString/Close errors on the token lock file — a partial write
  no longer strands an undeletable lock.
- store: a non-nil env map is now authoritative (hermetic) — a controlled map
  never falls back to ambient ZERO_OAUTH_* / HOME / XDG_CONFIG_HOME.
- scheduler: a transient loadForKey error backs off and retries (bounded) instead
  of permanently stopping proactive refresh.
- cli auth: reject flags a subcommand does not accept (e.g. login --watch,
  status --device) and reject empty --scope values — fail fast.

Lint blockers:
- manager: restructured the discovery fallback so the non-nil-err branch no longer
  returns nil (nilerr).
- providers: dropped the ineffectual flow initializer (ineffassign).

Tests: assert Save/Load errors in the scheduler test; the slow_down device test
no longer depends on post-expiry polling. Added tests for reserved-param
stripping, hermetic env, device default-expiry, and CLI flag validation. The
store-path test was already made OS-portable in the Windows-smoke fix.

Gates: gofmt/vet/build(host+linux+windows)/test -race/staticcheck(no new)/
govulncheck(0)/deadcode(no new) all pass.

* swarm: de-flake TestMailboxConcurrentSends on Windows CI

The 200-way concurrent-send stress test inherited from the swarm package timed
out on Windows CI (103/200 sends hit the 10s lock timeout): the lock's 20ms
retry sleep starves waiters when Windows file ops are slow under heavy
contention. Shorten the retry to 2ms (a freed lock is re-acquired promptly) and
raise the test's lock timeout to 60s so a legitimately-slow CI never times a send
out. Passes repeatedly under -race. (CI de-flake of a test that surfaced on
#210's Windows smoke; functionally unrelated to the OAuth changes.)

* oauth: address CodeRabbit re-review (4 findings)

- loopback (security): NewLoopbackListener refuses an empty CSRF state — an empty
  state would match a callback carrying no state at all, defeating the check.
- device (correctness): RequestDeviceCode now sends client_secret when set, so a
  confidential client authenticates on the device-authorization endpoint too
  (consistent with the token poll).
- cli auth: document --watch in the help Flags section.
- scheduler test: assert the seed Save error instead of ignoring it.

Also scrubbed a stray third-party reference from a loopback.go comment.
Tests added: empty-state rejection, device client_secret is sent. Gates:
gofmt/vet/build/test -race/staticcheck(no new)/govulncheck(0)/deadcode(no new)
all pass.

* mcp(oauth): delegate the OAuth engine to internal/oauth (dedup) (#212)

OAuth Phase 2: the MCP OAuth code now reuses the shared internal/oauth engine
instead of carrying its own copy of the transport/identity-agnostic primitives.
internal/mcp/oauth.go drops ~110 lines (526 -> 418): newPKCE, newState,
discoverAuthorizationServer, the authorize-URL builder, token exchange/refresh,
the token-response parser, and joinWellKnown now delegate to internal/oauth.

Behavior-preserving: MCP keeps its own OAuthConfig/StoredToken types,
LoginOptions/Login orchestration, loopback handling, registerClient (dynamic
client registration — not part of the shared engine), CLI output, and on-disk
token format (mcp-oauth-tokens.json, raw server-name keys). The existing mcp
oauth tests pass UNCHANGED. The only behavioral delta is a hardening inherited
from the shared engine: the token endpoint must be https (loopback exempt) —
a credential is no longer sent to a plaintext non-loopback endpoint.

Stacks on #210 (the internal/oauth package). Gates: gofmt/vet/build(host+
linux+windows)/test -race/staticcheck(no new)/govulncheck(0)/deadcode(no new
vs base) all pass; `zero mcp oauth status` output is identical.
 2026-06-15 18:47:43 +05:30