feat: add proxy auth mode for Atlassian CLIs

[niieani]

Summary

• Add proxy as a supported auth method for jtk and cfl
• Send no Authorization header in proxy mode and require only a URL
• Allow loopback http:// proxy URLs while continuing to reject arbitrary cleartext URLs
• Skip token/keyring requirements for proxy-mode runtime and init flows
• Add optional bearer gateway base URL overrides for controlled environments and tests
• Document proxy auth in READMEs, development notes, and integration test runbooks

Why

Some users run Atlassian CLI tools through a trusted local or managed proxy that handles upstream authentication. The existing basic and bearer modes require the CLI to own credentials and emit an auth header, which makes those proxy setups difficult or impossible to configure cleanly.

Implementation Notes

Proxy auth is explicit via ATLASSIAN_AUTH_METHOD=proxy, JIRA_AUTH_METHOD=proxy, or CFL_AUTH_METHOD=proxy. In this mode, the CLI constructs normal Jira or Confluence API URLs but deliberately omits Authorization. HTTPS proxy URLs are accepted, and http:// is limited to loopback hosts for local proxy workflows.

Bearer auth keeps existing behavior by default and adds ATLASSIAN_GATEWAY_BASE_URL, JIRA_GATEWAY_BASE_URL, and CFL_GATEWAY_BASE_URL for environments that need to override the Atlassian gateway base.

Tests

• go test ./... in shared
• go test ./... in tools/jtk
• go test ./... in tools/cfl

[piekstra-dev]

Automated PR Review

Reviewed commit: 2af24b9de893
Profile: reviewer - Posting as: piekstra-dev

Summary

Reviewer	Findings
go:implementation-tests	1
policies:conventions	1
structure:repo-health	1
documentation:docs	1

go:implementation-tests (1 finding)

Minor - tools/jtk/internal/cmd/initcmd/initcmd.go:318

api.New is now called unconditionally before the if !noVerify block, changing the semantics of --no-verify. Previously, the client was constructed only inside the verify branch, so --no-verify also bypassed client construction. For basic auth, api.New returns ErrAPITokenRequired when cfg.APIToken is empty. A user running jtk init --no-verify in interactive mode who completes the form without entering a token (intending to populate via set-credential later) will now get a client-construction failure where the old code would have saved the config successfully.

The test suite has no case asserting that basic-auth --no-verify can save a URL+email config when the token is absent, so this regression is invisible to CI.

Fix: move client construction back inside the if !noVerify block alongside the verification call, restoring the original invariant that --no-verify skips all client interaction. If eager config validation is the intent, add an explicit guard that skips api.New when noVerify && cfg.APIToken == "" for basic auth, and add a test documenting the allowed-to-save-without-token path.

policies:conventions (1 finding)

Major - tools/cfl/internal/cmd/init/init.go:464

The normalizeAuthConfig function introduced here is byte-for-byte identical to the one added in tools/jtk/internal/cmd/initcmd/initcmd.go. The logic it encodes — defaulting an empty auth_method to basic, and scrubbing Email/APIToken/CloudID for proxy auth — is auth-method policy, not tool-local behaviour. Duplicating it across two tool cmd packages means future auth method additions must be applied in two places. Move it to a shared package (e.g. shared/auth alongside ValidateAuthMethod, or a new shared/authconfig helper), expose it as a single exported function, and have both tools call it.

structure:repo-health (1 finding)

Major - tools/cfl/api/client.go:36

NewProxyClient skips the loopback-HTTP safety check that newProxyClient (jtk) enforces at the client boundary.

The invariant introduced by this diff is: proxy auth must never send unauthenticated requests to a non-loopback cleartext endpoint. tools/jtk/api/client.go:newProxyClient enforces this at client-construction time:

if strings.HasPrefix(baseURL, "http://") && !url.IsLoopbackHTTP(baseURL) {
    return nil, ErrProxyURLRequiresHTTPS
}

tools/cfl/api/client.go:NewProxyClient has no such check — it passes the URL straight to normalizeWikiBaseURL and client.New. The cfl path relies entirely on config.Validate() being called during cfl init, but Validate() is not called at runtime from root.go's APIClient(). A manually edited config (auth_method: proxy, url: http://external-host/...) would bypass validation and silently send unauthenticated requests to an arbitrary cleartext server.

Fix: add the identical guard in NewProxyClient before calling normalizeWikiBaseURL:

func NewProxyClient(baseURL string) (*Client, error) {
    normalized := normalizeWikiBaseURL(baseURL)
    if strings.HasPrefix(normalized, "http://") && !sharedurl.IsLoopbackHTTP(normalized) {
        return nil, ErrProxyURLRequiresHTTPS  // add this sentinel error
    }
    return &Client{Client: client.New(normalized, "", "", &client.Options{SkipAuthHeader: true})}, nil
}

This also makes the two tools' public API surfaces symmetric.

documentation:docs (1 finding)

Minor - tools/jtk/integration-tests.md

The PR updated cfl/integration-tests.md's Test Execution Checklist (line 592) to say "Run the full checklist with separate passes to ensure each auth path works" and covers all three methods. The equivalent section at the bottom of jtk/integration-tests.md was not updated — it almost certainly still says "Run the full checklist twice with separate passes to ensure both auth paths work." A tester following the jtk checklist would not know a third Proxy Auth pass is required. Add a "Pass 3: Proxy Auth" block and update the checklist preamble to match the updated cfl version.

0 PR discussion threads considered. 0 summarized; 0 resolved.

---

Completed in 8m 11s | ~$1.04 (est.) | claude-sonnet-4-6 | cr 0.4.161

Field	Value
Model	claude-sonnet-4-6
Reviewers	go:implementation-tests, policies:conventions, structure:repo-health, documentation:docs
Engine	claude_cli · claude-sonnet-4-6
Reviewed by	cr · piekstra-dev
Duration	8m 11s wall · 8m 10s compute
Cost	~$1.04 (est.)
Tokens	26 in / 10.7k out

Per-workstream usage

Workstream	Model	In	Out	Cache read	Cache create	Cost	Duration
orchestrator-selection	claude-sonnet-4-6	4	1.6k	0	20.5k	~$0.10 (est.)	26s
go:implementation-tests	claude-sonnet-4-6	6	1.1k	199.9k	76.7k	~$0.36 (est.)	3m 02s
policies:conventions	claude-sonnet-4-6	4	2.8k	7.2k	24.2k	~$0.14 (est.)	53s
structure:repo-health	claude-sonnet-4-6	4	2.9k	7.2k	27.6k	~$0.15 (est.)	53s
documentation:docs	claude-sonnet-4-6	4	1.3k	3.9k	44.5k	~$0.19 (est.)	1m 54s
orchestrator-rollup	claude-sonnet-4-6	4	975	24.1k	20.5k	~$0.10 (est.)	59s

[piekstra-dev]

Automated PR review completed with outcome: comment.

[niieani]

Hi @piekstra! I've fixed the PR based on the review and rebased. Thanks for taking a look.

[piekstra-dev]

Automated PR Review

Reviewed commit: ad44044b06b2
Profile: reviewer - Posting as: piekstra-dev

Summary

Requesting changes because trusted repo-local review guidance could not be loaded from .codereview/agents/ on the PR base branch.

Base branch .codereview/agents/ was not present for this review.

0 PR discussion threads considered. 0 summarized; 0 resolved.

[piekstra-dev]

Automated PR Review

Reviewed commit: 48422cba71a0
Profile: reviewer - Posting as: piekstra-dev

Summary

Reviewer	Findings
go:implementation-tests	0
policies:conventions	1
structure:repo-health	1
documentation:docs	1

policies:conventions (1 finding)

Minor - tools/cfl/internal/cmd/init/init.go:23

The bearer-auth (and basic-auth) form validators in cfl init always reject an empty API token regardless of --no-verify, while the equivalent jtk validators use if s == "" && !noVerify (initcmd.go:241, 298). jtk's finalizeInit also has an explicit cfg.APIToken == "" branch (initcmd.go:409-410) that emits a set-credential recovery hint, acknowledging the deferred-token workflow. cfl has no such branch and its form blocks the workflow entirely.

This asymmetry means jtk init --auth-method bearer --url ... --no-verify allows saving URL + Cloud ID now and adding the token later via set-credential, but cfl init --auth-method bearer --url ... --no-verify forces the user to supply a token at init time. Users who operate both tools will encounter inconsistent behavior for the same auth flow.

Fix: add && !noVerify to the bearer and basic token form validators in cfl init (mirroring jtk), and add a matching cfg.APIToken == "" branch in cfl finalizeInit that prints a cfl set-credential recovery hint so the deferred-token path is documented and tested.

structure:repo-health (1 finding)

Minor - tools/cfl/internal/cmd/init/init.go:23

requireNonInteractiveFields is structurally duplicated between tools/cfl/internal/cmd/init/init.go and tools/jtk/internal/cmd/initcmd/initcmd.go. Both functions encode the same auth-method branching policy (proxy → early return, bearer → require cloud-id, basic → require email + token) and differ only in the tool name embedded in the recovery hint. This is the same entropy pattern that produced the normalizeAuthConfig duplication fixed in this PR (now auth.NormalizeConfig). If a fourth auth method is added to one tool, the other silently keeps the old validation shape.

Fix: add an exported auth.RequireNonInteractiveFields(url, authMethod, email, apiToken, cloudID, toolHint string) error to shared/auth/auth.go alongside NormalizeConfig, taking the tool-specific credential command as a toolHint parameter for the recovery message. Both cfl init and jtk init then call it rather than maintaining parallel copies.

documentation:docs (1 finding)

Minor - tools/cfl/integration-tests.md:593

The Test Execution Checklist preamble (line 593) says "Run the full checklist with separate passes to ensure each auth path works," and the Auth Methods section at the top of the file enumerates three paths (Basic, Bearer, Proxy). However, only Pass 1 (Basic Auth) and Pass 2 (Bearer Auth) checklist blocks are present — there is no Pass 3 for Proxy Auth. A tester following this runbook would not know a proxy-auth pass is required.

The settled inline thread flagged this same gap in jtk/integration-tests.md, and that file was fixed with a full Pass 3 section (lines 1581–1604). cfl/integration-tests.md has the same structural gap and was not updated to match.

Fix: add a "Pass 3: Proxy Auth" block to the cfl Test Execution Checklist. At minimum it should mirror the jtk Pass 3 structure: setup steps (cfl init --auth-method proxy --url <proxy-url> --no-verify, cfl config show to verify auth_method=proxy and email/token absent, cfl space list to confirm the proxy path works), then a full checklist pass over all cfl command sections (all cfl commands have no scope restrictions, so no sections need to be skipped, unlike the jtk bearer pass).

Reviewer Coverage

Reviewer	Status	Inspected	Skipped	Constraints
go:implementation-tests	incomplete_skipped	shared/auth/auth.go, shared/auth/auth_test.go, shared/client/client.go, shared/client/client_test.go, shared/client/options.go, shared/url/url.go, shared/url/url_test.go, tools/cfl/api/client.go, tools/cfl/api/client_test.go, tools/cfl/internal/cmd/init/init.go, tools/cfl/internal/cmd/init/init_test.go, tools/cfl/internal/cmd/root/root.go, tools/cfl/internal/config/config.go, tools/jtk/api/client.go, tools/jtk/api/client_test.go, tools/jtk/internal/cmd/initcmd/initcmd.go, tools/jtk/internal/cmd/initcmd/initcmd_test.go, tools/jtk/internal/cmd/root/root.go	shared/credstore/conndivergence_test.go, shared/credstore/credstore.go, shared/credstore/credstore_test.go, tools/cfl/internal/cmd/root/root_test.go, tools/cfl/internal/config/config_test.go, tools/jtk/internal/cmd/root/root_test.go, tools/jtk/internal/config/config.go, tools/jtk/internal/config/config_test.go	unavailable
policies:conventions	complete_broad	shared/auth/auth.go, shared/client/client.go, shared/client/options.go, shared/credstore/credstore.go, tools/cfl/internal/cmd/init/init.go, tools/cfl/internal/cmd/root/root.go, tools/cfl/internal/config/config.go, tools/jtk/internal/cmd/initcmd/initcmd.go, tools/jtk/internal/cmd/root/root.go, tools/jtk/internal/config/config.go	unavailable	Shared CLI standards docs (cli-common/docs) and shared automation (.github) were not available locally; review is based solely on repo-local files and the visible diff context.; tools/cfl/api/client.go (the NewProxyClient implementation) is not in the assigned file list; the settled inline thread about missing loopback safety there could not be independently confirmed from assigned files.
structure:repo-health	complete_broad	shared/auth/auth.go, shared/client/client.go, shared/client/options.go, shared/credstore/credstore.go, shared/url/url.go, tools/cfl/api/client.go, tools/cfl/internal/cmd/init/init.go, tools/cfl/internal/cmd/root/root.go, tools/cfl/internal/config/config.go, tools/jtk/api/client.go, tools/jtk/internal/cmd/initcmd/initcmd.go, tools/jtk/internal/cmd/root/root.go, tools/jtk/internal/config/config.go	unavailable	Integration test runbook files (jtk/integration-tests.md, cfl/integration-tests.md) were not in the assigned file list and could not be verified for the proxy auth checklist parity finding raised in the settled thread.
documentation:docs	complete_broad	tools/cfl/README.md, tools/cfl/docs/development.md, tools/cfl/integration-tests.md, tools/jtk/README.md, tools/jtk/docs/development.md, tools/jtk/integration-tests.md	unavailable	Review limited to the six assigned changed files; code implementation files were not inspected.

0 PR discussion threads considered. 0 summarized; 0 resolved.

---

Completed in 5m 13s | ~$2.37 (est.) | claude-sonnet-4-6 | cr 0.10.243

Field	Value
Model	claude-sonnet-4-6
Reviewers	go:implementation-tests, policies:conventions, structure:repo-health, documentation:docs
Engine	claude_cli · claude-sonnet-4-6
Reviewed by	cr · piekstra-dev
Duration	5m 13s wall · 12m 16s compute
Cost	~$2.37 (est.)
Tokens	51 in / 38.9k out

Per-workstream usage

Workstream	Model	In	Out	Cache read	Cache create	Cost	Duration
orchestrator-selection	claude-sonnet-4-6	4	2.3k	28.7k	29.9k	~$0.15 (est.)	50s
go:implementation-tests	claude-sonnet-4-6	22	13.6k	903.6k	73.5k	~$0.75 (est.)	4m 39s
policies:conventions	claude-sonnet-4-6	8	10.6k	160.8k	58.7k	~$0.43 (est.)	3m 05s
structure:repo-health	claude-sonnet-4-6	7	6.8k	127.0k	73.5k	~$0.42 (est.)	1m 52s
documentation:docs	claude-sonnet-4-6	6	4.9k	131.2k	90.9k	~$0.45 (est.)	1m 22s
orchestrator-rollup	claude-sonnet-4-6	4	688	39.3k	37.6k	~$0.16 (est.)	25s

[niieani]

@piekstra-dev Follow-up review requested. Feedback from review 4704315391 is addressed in 2895893; all three inline threads have replies and are resolved. Please review the current head.

[piekstra-dev]

Automated PR Review

Reviewed commit: 2895893f83e5
Profile: reviewer - Posting as: piekstra-dev

Summary

Reviewer	Findings
go:implementation-tests	0
structure:repo-health	0
policies:conventions	0
documentation:docs	0

Reviewer Coverage

Reviewer	Status	Inspected	Skipped	Constraints
go:implementation-tests	incomplete_skipped	shared/auth/auth.go, shared/auth/auth_test.go, shared/client/client.go, shared/client/client_test.go, shared/client/options.go, shared/credstore/credstore.go, shared/url/url.go, tools/cfl/api/client.go, tools/cfl/api/client_test.go, tools/cfl/internal/cmd/init/init.go, tools/cfl/internal/cmd/root/root.go, tools/cfl/internal/config/config.go, tools/jtk/api/client.go, tools/jtk/internal/cmd/initcmd/initcmd.go, tools/jtk/internal/config/config.go	shared/credstore/conndivergence_test.go, shared/credstore/credstore_test.go, shared/url/url_test.go, tools/cfl/internal/cmd/init/finalize_test.go, tools/cfl/internal/cmd/init/init_test.go, tools/cfl/internal/cmd/root/root_test.go, tools/cfl/internal/config/config_test.go, tools/jtk/api/client_test.go, tools/jtk/internal/cmd/initcmd/initcmd_test.go, tools/jtk/internal/cmd/root/root.go, tools/jtk/internal/cmd/root/root_test.go, tools/jtk/internal/config/config_test.go	Did not run go test ./...; correctness of proxy-auth behavior was verified by reading implementation and test source, not by executing the suite.; Reviewed against the checked-out repo snapshot in the workbench rather than a raw diff; git history/blame was not consulted, so severity judgments rely on reading final file contents plus the PR dossier's discussion thread rather than confirming exact line-level diff hunks.
structure:repo-health	complete_broad	shared/auth/auth.go, shared/client/client.go, shared/client/options.go, shared/credstore/credstore.go, shared/url/url.go, tools/cfl/api/client.go, tools/cfl/internal/cmd/init/init.go, tools/cfl/internal/cmd/root/root.go, tools/cfl/internal/config/config.go, tools/jtk/api/client.go, tools/jtk/internal/cmd/initcmd/initcmd.go, tools/jtk/internal/cmd/root/root.go, tools/jtk/internal/config/config.go	unavailable	unavailable
policies:conventions	complete_broad	tools/cfl/README.md, tools/cfl/internal/cmd/init/init.go, tools/cfl/internal/cmd/root/root.go, tools/cfl/internal/config/config.go, tools/jtk/README.md, tools/jtk/internal/cmd/initcmd/initcmd.go, tools/jtk/internal/cmd/root/root.go, tools/jtk/internal/config/config.go	unavailable	Shared cli-common conventions docs were not present in the provided review context, so this review relies on cross-tool (cfl vs jtk) consistency and the diff/dossier discussion rather than the canonical standards text.; tools/cfl/api/client.go and tools/jtk/api/client.go (where the loopback-HTTP proxy guard and gateway base URL override live) are not in the assigned file set and were not inspected; the previously-discussed NewProxyClient asymmetry lives there, not in the assigned config/root/init files.
documentation:docs	complete_broad	tools/cfl/README.md, tools/cfl/docs/development.md, tools/cfl/integration-tests.md, tools/jtk/README.md, tools/jtk/docs/development.md, tools/jtk/integration-tests.md	unavailable	unavailable

0 PR discussion threads considered. 0 summarized; 0 resolved.

---

Completed in 4m 59s | unavailable | claude-sonnet-5 | cr 0.10.259

Field	Value
Model	claude-sonnet-5
Reviewers	go:implementation-tests, structure:repo-health, policies:conventions, documentation:docs
Engine	claude_cli · claude-sonnet-5
Reviewed by	cr · piekstra-dev
Duration	4m 59s wall · 8m 32s compute
Cost	unavailable
Tokens	unavailable

Per-workstream usage

Workstream	Model	In	Out	Cache read	Cache create	Cost	Duration
orchestrator-selection	claude-sonnet-5	unavailable	unavailable	unavailable	unavailable	unavailable	29s
go:implementation-tests	claude-sonnet-5	unavailable	unavailable	unavailable	unavailable	unavailable	2m 33s
structure:repo-health	claude-sonnet-5	unavailable	unavailable	unavailable	unavailable	unavailable	1m 34s
policies:conventions	claude-sonnet-5	unavailable	unavailable	unavailable	unavailable	unavailable	2m 03s
documentation:docs	claude-sonnet-5	unavailable	unavailable	unavailable	unavailable	unavailable	1m 39s
orchestrator-rollup	claude-sonnet-5	unavailable	unavailable	unavailable	unavailable	unavailable	11s

[piekstra-dev]

Automated PR Review

Reviewed commit: d58dd28583d9
Profile: reviewer - Posting as: piekstra-dev

Summary

Reviewer	Findings
go:implementation-tests	0
documentation:docs	0
policies:conventions	0
structure:repo-health	0
security:code-auditor	0
architecture:solid	5

architecture:solid (5 findings)

Major - tools/jtk/api/client.go:107

U-D1: the gateway base URL is reached for ambiently inside the constructor instead of arriving as a dependency.

client.GatewayBaseURLFromEnv("JIRA_GATEWAY_BASE_URL") (and the same call at tools/cfl/api/client.go:58) reads os.Getenv deep inside client construction. ClientConfig sits three lines away and already carries every other connection input explicitly (URL, Email, APIToken, AuthMethod, CloudID), so this one input is invisible in the signature: api.New(cfg) can now target different hosts for identical cfg values depending on process environment, and tests must mutate global env (t.Setenv) to exercise it — which is precisely the seam U-D1 says should be a parameter. The PR intent names "controlled environments and tests" as the motivation, which makes injection the cheaper design, not the more expensive one.

A second, related gap on the same lines: the override is applied with no scheme check, while this PR is otherwise careful about cleartext (line 131 rejects non-loopback http:// for proxy). JIRA_GATEWAY_BASE_URL=http://host sends a Bearer token in cleartext to an arbitrary host with no diagnostic.

Suggested fix: add GatewayBaseURL string to api.ClientConfig (and a parameter or small config struct for cfl's NewBearerClient), resolve the env value once in the composition root — jtk root.Options.APIClient() / cfl root.Options.APIClient(), where the rest of the env-derived config is already assembled — and pass it down. Keep client.GatewayBaseURLFromEnv as the resolver called from that one place, and reuse the proxy scheme predicate to reject non-loopback cleartext overrides.

Major - tools/cfl/api/client.go:41

U-S1 / U-G1: the proxy cleartext-URL invariant is restated in three places with two sentinel errors, and the restatements already differ.

The new rule "http:// is allowed only for loopback" now appears at:

• tools/cfl/api/client.go:41 — strings.HasPrefix(normalized, "http://") && !sharedurl.IsLoopbackHTTP(normalized) → cfl's own ErrProxyURLRequiresHTTPS (line 21)
• tools/jtk/api/client.go:131 — byte-identical predicate → jtk's own ErrProxyURLRequiresHTTPS (line 169)
• tools/cfl/internal/config/config.go:71-75 — !strings.HasPrefix(c.URL, "https://") guarded by AuthMethod != proxy || !IsLoopbackHTTP(...), with a third error string

shared/url.IsLoopbackHTTP shares only the host-classification primitive; the policy is copied. The third form is not equivalent to the other two: a scheme-less URL (mycompany.atlassian.net) is rejected by the config form but normalized to https and accepted by the api form — so the phrasings have already drifted. This is the same duplication the settled normalizeAuthConfig thread pushed into shared/auth; the safety predicate is the piece that did not make the trip, and it is the one where drift means an unauthenticated request to an arbitrary cleartext host.

Suggested fix: add one exported predicate next to the other auth policy — e.g. func ValidateProxyURL(u string) error in shared/auth (or RequireSecureOrLoopback in shared/url) returning a single shared sentinel ErrProxyURLRequiresHTTPS. Have both NewProxyClients and cfl's config.validate call it, and keep per-tool sentinels only as aliases if the existing exported names must stay for compatibility.

Major - tools/cfl/internal/cmd/init/init.go:33

U-O1: cfl's auth-method dispatch is duplicated per call site, while jtk's lives in one place.

jtk exposes a single constructor — api.New(ClientConfig) (tools/jtk/api/client.go:50) switches on AuthMethod internally, so adding proxy touched exactly one dispatch site and every caller (root APIClient(), initcmd's defaultClientBuilder) got it for free. cfl instead exports three constructors and repeats the ladder in each consumer: defaultClientBuilder here (lines 29-37) and Options.APIClient() in tools/cfl/internal/cmd/root/root.go:144-150. This PR had to edit both, and the divergence that the settled thread on NewProxyClient caught (missing loopback guard) is exactly the failure mode this shape produces: two hand-copied ladders that can disagree.

Suggested fix: give cfl the same single entry point as jtk — an api.ClientConfig{URL, Email, APIToken, AuthMethod, CloudID} plus api.New(cfg) (*Client, error) that owns the switch, keeping NewClient/NewBearerClient/NewProxyClient as unexported (or thin, test-only) constructors. Then defaultClientBuilder becomes api.New(...) and root.go's APIClient() collapses to one call, so the next auth method is a one-site change in both tools.

Minor - shared/auth/auth.go:38

U-I1 / U-G1: NormalizeConfig takes four same-typed strings and returns four same-typed strings, positionally.

cfg.AuthMethod, cfg.Email, cfg.APIToken, cfg.CloudID = auth.NormalizeConfig(
    cfg.AuthMethod, cfg.Email, cfg.APIToken, cfg.CloudID,
)

(tools/cfl/internal/cmd/init/init.go:339 and tools/jtk/internal/cmd/initcmd/initcmd.go:330). Any transposition — at a call site or in the return assignment — compiles silently and misroutes credential fields; the failure would look like "email lost, cloud ID holds a token". RequireNonInteractiveFields has the same shape with six strings. This is brand-new shared API with two consumers today and an explicit intent to grow (a third tool, or a fourth auth method), so the shape is worth fixing before it spreads.

Suggested fix: give the shared package the value it is actually operating on, e.g.

type Credentials struct{ Method, Email, APIToken, CloudID string }
func (c Credentials) Normalize() Credentials
func (c Credentials) RequireNonInteractive(url, toolHint string) error

Callers then build one struct from their config and assign back field-by-field with names visible, and adding a fifth policy field is additive rather than a signature break at every call site.

Minor - tools/jtk/internal/cmd/initcmd/initcmd.go:330

U-L1: the two init commands present the same documented contract but validate asymmetrically — jtk has no validation gate at all.

cfl normalizes and then gates on cfg.ValidateForInit(noVerify) (tools/cfl/internal/cmd/init/init.go:343) before saving. jtk normalizes here and goes straight to finalizeInit, where the only validation is api.New — which finalizeInit constructs inside if !noVerify (line 348-351). So the exact invocation this PR adds to the help text at line 80, jtk init --auth-method proxy --url http://127.0.0.1:8080/atlassian --no-verify, runs with zero URL-scheme checking: substitute a non-loopback cleartext host and jtk writes it into the shared credential store (which cfl also reads) and prints "Configuration saved… / Try it out", with the failure deferred to the next command. jtk's config package has no Validate at all, so there is nothing to call today.

Mitigating fact, stated for calibration: no unauthenticated cleartext request escapes — newProxyClient (tools/jtk/api/client.go:131) and, after the settled thread, NewProxyClient (tools/cfl/api/client.go:41) both reject the URL at runtime. The cost here is a save that reports success and a bad value in shared state, not a leak.

Suggested fix: call the shared proxy-URL predicate (see the shared/auth/shared/url extraction suggested on tools/cfl/api/client.go:41) right after auth.NormalizeConfig here, so both tools reject the URL at the same point in the init flow, and add a jtk init test covering --auth-method proxy --no-verify with a non-loopback http:// URL asserting nothing is written.

Reviewer Coverage

• go:implementation-tests — ⚠️ incomplete (skipped files); inspected 16 assigned files (24 inspected across reviewers): shared/auth/auth.go, shared/auth/auth_test.go, shared/client/client.go, shared/client/client_test.go, shared/client/options.go, shared/credstore/credstore.go, shared/url/url.go, shared/url/url_test.go, tools/cfl/api/client.go, tools/cfl/api/client_test.go, tools/cfl/internal/cmd/init/init.go, tools/cfl/internal/cmd/root/root.go, tools/cfl/internal/config/config.go, tools/jtk/api/client.go, tools/jtk/api/client_test.go, tools/jtk/internal/cmd/initcmd/initcmd.go; skipped: shared/credstore/conndivergence_test.go, shared/credstore/credstore_test.go, tools/cfl/internal/cmd/configcmd/show_test.go, tools/cfl/internal/cmd/init/finalize_test.go, tools/cfl/internal/cmd/init/init_test.go, tools/cfl/internal/cmd/root/root_test.go, tools/cfl/internal/config/config_test.go, tools/jtk/internal/cmd/initcmd/initcmd_test.go, tools/jtk/internal/cmd/root/root.go, tools/jtk/internal/cmd/root/root_test.go, tools/jtk/internal/config/config.go, tools/jtk/internal/config/config_test.go; constraints: Files outside the assigned list (READMEs, integration-tests.md, docs/development.md) were not evaluated even though the PR discussion references them. Reviewed against the working-tree checkout provided in the reviewer workbench (artifact-clone), not via direct git diff commands.
• documentation:docs — complete (broad); inspected 6 assigned files (24 inspected across reviewers): tools/cfl/README.md, tools/cfl/docs/development.md, tools/cfl/integration-tests.md, tools/jtk/README.md, tools/jtk/docs/development.md, tools/jtk/integration-tests.md; skipped: none; constraints: none
• policies:conventions — complete (broad); inspected 12 assigned files (24 inspected across reviewers): shared/auth/auth.go, shared/client/options.go, shared/credstore/credstore.go, shared/url/url.go, tools/cfl/README.md, tools/cfl/internal/cmd/init/init.go, tools/cfl/internal/cmd/root/root.go, tools/cfl/internal/config/config.go, tools/jtk/README.md, tools/jtk/internal/cmd/initcmd/initcmd.go, tools/jtk/internal/cmd/root/root.go, tools/jtk/internal/config/config.go; skipped: none; constraints: none
• structure:repo-health — complete (broad); inspected 11 assigned files (24 inspected across reviewers): shared/auth/auth.go, shared/client/client.go, shared/client/options.go, shared/credstore/credstore.go, shared/url/url.go, tools/cfl/api/client.go, tools/cfl/internal/cmd/init/init.go, tools/cfl/internal/config/config.go, tools/jtk/api/client.go, tools/jtk/internal/cmd/initcmd/initcmd.go, tools/jtk/internal/config/config.go; skipped: none; constraints: none
• security:code-auditor — complete (broad); inspected 9 assigned files (24 inspected across reviewers): shared/auth/auth.go, shared/client/client.go, shared/client/options.go, shared/credstore/credstore.go, shared/url/url.go, tools/cfl/api/client.go, tools/cfl/internal/cmd/init/init.go, tools/jtk/api/client.go, tools/jtk/internal/cmd/initcmd/initcmd.go; skipped: none; constraints: none
• architecture:solid — complete (broad); inspected 5 assigned files (24 inspected across reviewers): shared/auth/auth.go, tools/cfl/api/client.go, tools/cfl/internal/cmd/init/init.go, tools/jtk/api/client.go, tools/jtk/internal/cmd/initcmd/initcmd.go; skipped: none; constraints: Could not run the project's verification (make check / go test ./...): command execution was denied by the sandbox in this environment. No finding here is based on a red check. Findings are limited to the five assigned files; shared/client/options.go, shared/url/url.go, tools/*/internal/cmd/root/root.go and tools/cfl/internal/config/config.go were read only as context and are cited as evidence, not anchored. git was unavailable in this environment, so changed-vs-unchanged lines were inferred from the supplied change map plus file content rather than from the actual diff hunks.

Inspected files (24)

• shared/auth/auth.go
• shared/auth/auth_test.go
• shared/client/client.go
• shared/client/client_test.go
• shared/client/options.go
• shared/credstore/credstore.go
• shared/url/url.go
• shared/url/url_test.go
• tools/cfl/README.md
• tools/cfl/api/client.go
• tools/cfl/api/client_test.go
• tools/cfl/docs/development.md
• tools/cfl/integration-tests.md
• tools/cfl/internal/cmd/init/init.go
• tools/cfl/internal/cmd/root/root.go
• tools/cfl/internal/config/config.go
• tools/jtk/README.md
• tools/jtk/api/client.go
• tools/jtk/api/client_test.go
• tools/jtk/docs/development.md
• tools/jtk/integration-tests.md
• tools/jtk/internal/cmd/initcmd/initcmd.go
• tools/jtk/internal/cmd/root/root.go
• tools/jtk/internal/config/config.go

0 PR discussion threads considered. 0 summarized; 0 resolved.

---

Completed in 7m 55s | $5.33 | claude-sonnet-5, claude-opus-5 | cr 0.10.289

Field	Value
Model	claude-sonnet-5, claude-opus-5
Reviewers	go:implementation-tests, documentation:docs, policies:conventions, structure:repo-health, security:code-auditor, architecture:solid
Engine	claude_cli · claude-sonnet-5, claude-opus-5
Reviewed by	cr · piekstra-dev
Duration	7m 55s wall · 12m 58s compute
Cost	$5.33
Tokens	204 in / 52.2k out

Per-workstream usage

• orchestrator-selection — claude-sonnet-5
  • In: 6
  • Out: 6.2k
  • Cache read: 81.4k
  • Cache create: 27.7k
  • Cost: $0.19
  • Duration: 58s
• go:implementation-tests — claude-sonnet-5
  • In: 38
  • Out: 6.4k
  • Cache read: 964.0k
  • Cache create: 62.9k
  • Cost: $0.51
  • Duration: 1m 35s
• documentation:docs — claude-sonnet-5
  • In: 28
  • Out: 5.2k
  • Cache read: 1.1M
  • Cache create: 140.2k
  • Cost: $0.84
  • Duration: 1m 17s
• policies:conventions — claude-sonnet-5
  • In: 28
  • Out: 5.6k
  • Cache read: 664.9k
  • Cache create: 78.1k
  • Cost: $0.50
  • Duration: 1m 36s
• structure:repo-health — claude-sonnet-5
  • In: 20
  • Out: 3.3k
  • Cache read: 394.2k
  • Cache create: 56.2k
  • Cost: $0.34
  • Duration: 56s
• security:code-auditor — claude-sonnet-5
  • In: 28
  • Out: 3.7k
  • Cache read: 558.7k
  • Cache create: 65.9k
  • Cost: $0.41
  • Duration: 1m 03s
• architecture:solid — claude-opus-5
  • In: 50
  • Out: 21.0k
  • Cache read: 1.5M
  • Cache create: 103.7k
  • Cost: $2.31
  • Duration: 4m 49s
• orchestrator-rollup — claude-sonnet-5
  • In: 6
  • Out: 683
  • Cache read: 142.3k
  • Cache create: 48.4k
  • Cost: $0.23
  • Duration: 40s

[piekstra-dev]

U-O1: cfl's auth-method dispatch is duplicated per call site, while jtk's lives in one place.

jtk exposes a single constructor — api.New(ClientConfig) (tools/jtk/api/client.go:50) switches on AuthMethod internally, so adding proxy touched exactly one dispatch site and every caller (root APIClient(), initcmd's defaultClientBuilder) got it for free. cfl instead exports three constructors and repeats the ladder in each consumer: defaultClientBuilder here (lines 29-37) and Options.APIClient() in tools/cfl/internal/cmd/root/root.go:144-150. This PR had to edit both, and the divergence that the settled thread on NewProxyClient caught (missing loopback guard) is exactly the failure mode this shape produces: two hand-copied ladders that can disagree.

Suggested fix: give cfl the same single entry point as jtk — an api.ClientConfig{URL, Email, APIToken, AuthMethod, CloudID} plus api.New(cfg) (*Client, error) that owns the switch, keeping NewClient/NewBearerClient/NewProxyClient as unexported (or thin, test-only) constructors. Then defaultClientBuilder becomes api.New(...) and root.go's APIClient() collapses to one call, so the next auth method is a one-site change in both tools.

Reply inline to this comment.

[piekstra-dev]

U-L1: the two init commands present the same documented contract but validate asymmetrically — jtk has no validation gate at all.

cfl normalizes and then gates on cfg.ValidateForInit(noVerify) (tools/cfl/internal/cmd/init/init.go:343) before saving. jtk normalizes here and goes straight to finalizeInit, where the only validation is api.New — which finalizeInit constructs inside if !noVerify (line 348-351). So the exact invocation this PR adds to the help text at line 80, jtk init --auth-method proxy --url http://127.0.0.1:8080/atlassian --no-verify, runs with zero URL-scheme checking: substitute a non-loopback cleartext host and jtk writes it into the shared credential store (which cfl also reads) and prints "Configuration saved… / Try it out", with the failure deferred to the next command. jtk's config package has no Validate at all, so there is nothing to call today.

Mitigating fact, stated for calibration: no unauthenticated cleartext request escapes — newProxyClient (tools/jtk/api/client.go:131) and, after the settled thread, NewProxyClient (tools/cfl/api/client.go:41) both reject the URL at runtime. The cost here is a save that reports success and a bad value in shared state, not a leak.

Suggested fix: call the shared proxy-URL predicate (see the shared/auth/shared/url extraction suggested on tools/cfl/api/client.go:41) right after auth.NormalizeConfig here, so both tools reject the URL at the same point in the init flow, and add a jtk init test covering --auth-method proxy --no-verify with a non-loopback http:// URL asserting nothing is written.

Reply inline to this comment.

[piekstra-dev]

U-S1 / U-G1: the proxy cleartext-URL invariant is restated in three places with two sentinel errors, and the restatements already differ.

The new rule "http:// is allowed only for loopback" now appears at:

• tools/cfl/api/client.go:41 — strings.HasPrefix(normalized, "http://") && !sharedurl.IsLoopbackHTTP(normalized) → cfl's own ErrProxyURLRequiresHTTPS (line 21)
• tools/jtk/api/client.go:131 — byte-identical predicate → jtk's own ErrProxyURLRequiresHTTPS (line 169)
• tools/cfl/internal/config/config.go:71-75 — !strings.HasPrefix(c.URL, "https://") guarded by AuthMethod != proxy || !IsLoopbackHTTP(...), with a third error string

shared/url.IsLoopbackHTTP shares only the host-classification primitive; the policy is copied. The third form is not equivalent to the other two: a scheme-less URL (mycompany.atlassian.net) is rejected by the config form but normalized to https and accepted by the api form — so the phrasings have already drifted. This is the same duplication the settled normalizeAuthConfig thread pushed into shared/auth; the safety predicate is the piece that did not make the trip, and it is the one where drift means an unauthenticated request to an arbitrary cleartext host.

Suggested fix: add one exported predicate next to the other auth policy — e.g. func ValidateProxyURL(u string) error in shared/auth (or RequireSecureOrLoopback in shared/url) returning a single shared sentinel ErrProxyURLRequiresHTTPS. Have both NewProxyClients and cfl's config.validate call it, and keep per-tool sentinels only as aliases if the existing exported names must stay for compatibility.

Reply inline to this comment.

[piekstra-dev]

U-I1 / U-G1: NormalizeConfig takes four same-typed strings and returns four same-typed strings, positionally.

cfg.AuthMethod, cfg.Email, cfg.APIToken, cfg.CloudID = auth.NormalizeConfig(
    cfg.AuthMethod, cfg.Email, cfg.APIToken, cfg.CloudID,
)

(tools/cfl/internal/cmd/init/init.go:339 and tools/jtk/internal/cmd/initcmd/initcmd.go:330). Any transposition — at a call site or in the return assignment — compiles silently and misroutes credential fields; the failure would look like "email lost, cloud ID holds a token". RequireNonInteractiveFields has the same shape with six strings. This is brand-new shared API with two consumers today and an explicit intent to grow (a third tool, or a fourth auth method), so the shape is worth fixing before it spreads.

Suggested fix: give the shared package the value it is actually operating on, e.g.

type Credentials struct{ Method, Email, APIToken, CloudID string }
func (c Credentials) Normalize() Credentials
func (c Credentials) RequireNonInteractive(url, toolHint string) error

Callers then build one struct from their config and assign back field-by-field with names visible, and adding a fifth policy field is additive rather than a signature break at every call site.

Reply inline to this comment.

[piekstra-dev]

U-D1: the gateway base URL is reached for ambiently inside the constructor instead of arriving as a dependency.

client.GatewayBaseURLFromEnv("JIRA_GATEWAY_BASE_URL") (and the same call at tools/cfl/api/client.go:58) reads os.Getenv deep inside client construction. ClientConfig sits three lines away and already carries every other connection input explicitly (URL, Email, APIToken, AuthMethod, CloudID), so this one input is invisible in the signature: api.New(cfg) can now target different hosts for identical cfg values depending on process environment, and tests must mutate global env (t.Setenv) to exercise it — which is precisely the seam U-D1 says should be a parameter. The PR intent names "controlled environments and tests" as the motivation, which makes injection the cheaper design, not the more expensive one.

A second, related gap on the same lines: the override is applied with no scheme check, while this PR is otherwise careful about cleartext (line 131 rejects non-loopback http:// for proxy). JIRA_GATEWAY_BASE_URL=http://host sends a Bearer token in cleartext to an arbitrary host with no diagnostic.

Suggested fix: add GatewayBaseURL string to api.ClientConfig (and a parameter or small config struct for cfl's NewBearerClient), resolve the env value once in the composition root — jtk root.Options.APIClient() / cfl root.Options.APIClient(), where the rest of the env-derived config is already assembled — and pass it down. Keep client.GatewayBaseURLFromEnv as the resolver called from that one place, and reuse the proxy scheme predicate to reject non-loopback cleartext overrides.

Reply inline to this comment.