Repository navigation
Conversation
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. |
|
Superseded by open-cli-collective#502 so both ordered PRs are visible upstream. Merge upstream open-cli-collective#501 first, then rebase and merge open-cli-collective#502. |
![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: e0bde24ff1
ℹ️ 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 c.APIToken == "" { | ||
| return fmt.Errorf("--non-interactive: missing required value for --token-stdin or --token-from-env VAR (or pre-stage with `%s`)", toolHint) |
There was a problem hiding this comment.
When init runs with --no-verify and non-interactive or piped stdin, this unconditional token check rejects an empty token before either CLI reaches its new ValidateForInit(noVerify) and empty-token save paths. Consequently, deferred credential setup works interactively but commands such as jtk init --non-interactive --no-verify --url ... --email ... still fail, which prevents the intended scripted workflow of saving configuration and staging the token later. Pass the verification policy into this check or otherwise waive only the token requirement when verification is disabled.
Useful? React with 👍 / 👎.
| if err := url.RequireSecureOrLoopback(cfg.URL); err != nil { | ||
| return nil, err | ||
| } | ||
| baseURL := url.NormalizeURL(cfg.URL) | ||
| restURL := baseURL + "/rest/api/3" |
There was a problem hiding this comment.
When a proxy URL contains userinfo such as https://user:secret@proxy.example, the secure-URL check accepts it and normalization preserves it; Go's net/http then synthesizes a Basic Authorization header from that userinfo even though SkipAuthHeader is set. This violates proxy mode's no-Authorization contract and also persists the embedded password in the non-secret URL field. Both proxy client constructors should reject URLs whose parsed User is non-nil.
Useful? React with 👍 / 👎.
Adds explicit
proxyauth forjtkandcfl: only a URL is required, requests send no Authorization header, and token/keyring requirements are skipped. HTTPS and loopback HTTP endpoints are accepted; remote HTTP is rejected during client construction and init, includingjtk init --no-verifybefore any config is saved.Merge order: 2 of 2 — merge upstream open-cli-collective#501 first. This PR is stacked on
bb/bearer-gatewayin the niieani fork because GitHub cannot use a fork branch as the base of an upstream PR. After open-cli-collective#501 merges, rebase this branch onto upstreammainand open the proxy-auth PR upstream. Do not merge this into the fork's gateway branch before open-cli-collective#501 lands; that would enlarge open-cli-collective#501's diff.First PR: open-cli-collective#501
This replaces the proxy-auth portion of upstream open-cli-collective#411. It includes typed shared credential policy, consistent init validation and deferred-token behavior, both runtime paths, and all three auth passes in the integration runbooks. Existing tests remain in their original files to keep the review diff focused.
Validation:
make checkpassed on the complete stack (module tidy, lint, race tests for all three modules, and both CLI builds). Coverage includes omitted Authorization headers, proxy config persistence without credentials, non-loopback HTTP rejection before save, and existing basic/bearer behavior.