Repository navigation
Conversation
HEY's API is about to be served at api.hey.com while signing in stays at app.hey.com, whose tokens api.hey.com reads from the shared tables. The CLI keys everything on one base URL, so pointing it at api.hey.com lost the login and took the next one from an issuer it should not use. api_url (HEY_API_URL) moves what the SDK talks to and nothing else. Signing in, refreshing, the credential key, /cable and draft links stay on the base URL. Saved, it is keyed by login origin as account_defaults is, so no other server's token goes there, and a repository's .hey/config.json has no say in it.
A 307 or 308 from /oauth/tokens made Go resend the form, refresh token or code in it, to wherever the Location named. With an API origin beside the login origin that is a live way for a refresh token to leave the issuer. The SDK's own OAuth client already follows none, for the same reason. The 3xx is now the answer, an error that leaves the login in place.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
A review of this branch aimed at a token at the wrong origin, a refresh at the API origin and credentials under the wrong key raised four things. Three are fixed, one is declined.
Found sound in the same review: the credential lookup stays on the base URL, the SDK strips bearer and cookie on an off-origin redirect and doesn't refresh on that hop's 401, repository config can't supply the API origin, and |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 14 files
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
There was a problem hiding this comment.
🟡 Changes recommended
API URL validation still accepts empty query/fragment delimiters and encoded paths, allowing SDK blob/image requests to target unintended resources instead of enforcing origin-only endpoints.
1 open issue
What changed in this PR
Separates the SDK’s API origin from the login origin, allowing hey-cli to use api.hey.com while keeping authentication, credential storage, and browser links on the base URL.
Changes:
- Adds validated, login-origin-scoped
api_urlconfiguration and theHEY_API_URLoverride. - Routes SDK requests to the API origin and exposes it in configuration and authentication status.
- Blocks token-endpoint redirects and adds tests for routing, refresh, credential isolation, and pagination.
[!TIP]
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or rungh pr ready --undo.
Click "Ready for review" or rungh pr readyto reengage.
| File | Description |
|---|---|
internal/config/config.go |
Adds API origin loading, validation, persistence, and precedence. |
internal/config/config_test.go |
Tests origin scoping, overrides, validation, and persistence. |
internal/cmd/sdk.go |
Configures the SDK with the API origin. |
internal/cmd/root.go |
Passes the effective API origin to SDK initialization. |
internal/cmd/help_topics.go |
Documents HEY_API_URL in environment help. |
internal/cmd/draft.go |
Clarifies in a comment that draft links use the base URL. |
internal/cmd/config.go |
Adds API URL setting and display support. |
internal/cmd/auth.go |
Includes the configured API URL in authentication status. |
internal/cmd/api_origin_test.go |
Exercises separate login and API origins through real commands. |
internal/auth/auth.go |
Prevents token-endpoint redirects. |
internal/auth/auth_test.go |
Tests redirect refusal during refresh and sign-in. |
docs/cli.md |
Documents API origin configuration and reset behavior. |
docs/architecture/auth-and-setup.md |
Explains routing and credential boundaries. |
AGENTS.md |
Records API origin and storage guidance. |
Review effort: Balanced
A malformed api_urls key in a repository's .hey/config.json failed loading even though that file has no say in them, so they are normalized for the global file only. Two keys for one login origin are now refused, as account_defaults are, rather than one winning by map order. An API URL can no longer carry an empty query or fragment, or a path behind an escape. The docs say what HEY_TOKEN and HEY_API_URL do beside the setting.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 993792a44d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if env := os.Getenv("HEY_API_URL"); env != "" { | ||
| if apiErr := validateAPIURL(env); apiErr != nil { | ||
| return nil, apiErr |
There was a problem hiding this comment.
When HEY_API_URL is malformed and the invocation supplies --base-url, config.Load rejects the environment value before the root command applies the flag. The command therefore exits even though --base-url is documented to ignore HEY_API_URL in docs/cli.md:67; skip or defer this validation when the explicit base-URL flag will discard the setting.
Useful? React with 👍 / 👎.
| func validateAPIURL(apiURL string) error { | ||
| u, err := parseServerURL("API URL", apiURL) | ||
| if err != nil { |
There was a problem hiding this comment.
validateAPIURL delegates scheme validation to a helper that exempts localhost from every scheme check, so values such as ftp://localhost or file://localhost are accepted and persisted. The SDK uses Go's HTTP client and cannot issue requests with those schemes, leaving every data command to fail at runtime; the localhost exception should allow plain http, not arbitrary protocols.
Useful? React with 👍 / 👎.
| // that credential any more than it outlives a logout. | ||
| authMgr.OnCredentialCleared(func() { clearHTTPCache(cmd.ErrOrStderr()) }) | ||
| initSDK(authMgr, cfg.BaseURL) | ||
| initSDK(authMgr, cfg.APIOrigin()) |
There was a problem hiding this comment.
When a separate API origin returns an absolute Location for a draft save, routing the MCP SDK through cfg.APIOrigin() causes internal/mcpserver/drafts.go:draftLink to expose an app_url on that API host. That host serves API responses rather than the browser draft page, so a person following the MCP result gets a broken link; build this link from cfg.BaseURL, as the regular draft commands do.
AGENTS.md reference: AGENTS.md:L43-L45
Useful? React with 👍 / 👎.
Deliverable: https://app.basecamp.com/2914079/buckets/48804061/todos/10477329816
Rung H1 of hey2's launch is one staff identity's CLI reading at api.hey.com, with a token from app.hey.com's issuer, which hey2 verifies from the shared tables. The CLI keys everything on one base URL, so
HEY_BASE_URL=https://api.hey.comloses the login and takes the next one from an issuer it shouldn't use.What it does
api_url(HEY_API_URL, orhey config set api_url) moves the SDK's requests and nothing else.hey.Config.BaseURLiscfg.APIOrigin(); everything else still readscfg.BaseURL.hey mcp's includedhey auth login, the code exchangeTokenEndpoint, else base URL/cabledraftAppURL, a page a person opensWith no
api_url, nothing changes, in output included.Saved, it's
api_urlsin the globalconfig.json, keyed by login origin asaccount_defaultsis, so a token from another server never goes to it: not underHEY_BASE_URL,--base-urlor a repository's trustedbase_url.--base-urldropsHEY_API_URLtoo. A repository's.hey/config.jsoncan't set it at all. It must be an origin: HTTPS or localhost, with no path, query or userinfo.A 401 from the API origin refreshes at the login origin and resends once. A 401 that persists ends as an auth failure with the login kept;
invalid_grantat the login origin clears it. hey2's challenge naming its own issuer is ignored, since the CLI does no discovery. hey2'snot_admitted403 refreshes nothing.Absolute URLs from the server. No SDK change. The SDK strips credentials on a redirect off the origin and doesn't take a 401 from that hop as its own.
next_history_url,posting_changes_urland a label'sLinkgive up only their cursor, asked of the API origin. Attachments are relative paths only, and a blob on another origin is refused.The second commit is older than this feature:
/oauth/tokensanswering a 307 or 308 made Go resend the form, refresh token included, to theLocation. The Manager's client now follows none, as the SDK's own OAuth client doesn't. A 3xx is an error that leaves the login in place. Any redirect there was already wrong, so this changes nothing for a working server.What it doesn't
base_urldoes today; the setting is the owner's own, and no repository file can set it./cablestays on the base URL, withHEY_CABLE_URLstill overriding it.hey watchtakes its doorbell from app.hey.com and reads its changes from the API origin.api_urlset. The fetcher refuses them, so the TUI draws no image. It fails closed, and reaching them would take a second SDK client, which H1 doesn't need.hey mcp's draftapp_urlcomes from theLocationHEY answers with, so at api.hey.com it would name a page there that doesn't exist. It's a write, so it's H2's, and worth settling before then.hey draft's own link uses the base URL and is right.Tests
internal/cmd/api_origin_test.goruns the real command tree against two fake servers, a login origin and an API origin, each recording everything it hears:HEY_TOKEN)./oauth.hey auth login --no-browseris driven to the end by answering its callback.Pointing
initSDKback atBaseURL, or building the Manager fromAPIOrigin(), fails most of these.TestTokenEndpointRedirectsAreNotFollowedfails without the second commit.Run here:
make fmt-check vet test-unit test-sync-skills tidy-check check-surface check-release-lockstep, and golangci-lint v2.14.0 with 0 issues.make lintcan't find golangci-lint under this repo's.mise.tomlon this machine, so I ran the pinned binary directly. Nothing here has run against the real api.hey.com, which isn't serving yet (hey2 #1315 is open).Summary by cubic
Adds an
api_urlsetting that sends the SDK's API requests to a separate origin — api.hey.com for hey2's launch — while signing in, token refresh, credential storage,/cable, and draft links stay at the base URL. With noapi_urlset, nothing changes.Details
HEY_API_URLorhey config set api_url <origin>routes every API request there, and a 401 from it refreshes at the login origin exactly once before failing.HEY_BASE_URL,--base-url, or a repository's.hey/config.json— never goes there; conflicting keys for one login origin are refused, and a repository's file can't set it or fail loading on a bad one.Written for commit 993792a. Summary will update on new commits.