fix(security): scope API redirects and sanitize diagnostics

[steipete]

The standard HTTP client forwards custom X-Goog-Api-Key headers across redirects, and upstream/transport errors can echo that key. Parser diagnostics also bypassed the CLI's terminal sanitizer. These are separate entry paths into the same credential and diagnostic boundaries.

Apply a per-request redirect policy that preserves the caller's HTTP client, custom stop policy, and default ten-hop limit while rejecting changes of scheme, hostname, or effective port. Centralize request-error formatting to redact the configured key while retaining error causes for errors.Is/errors.As; redact APIError bodies as well. Route parser diagnostics through the existing sanitizer. The client notes and Unreleased changelog document the security tightening.

Regression tests first failed on the old code for cross-origin forwarding, API/transport/invalid-URL key exposure, and parser control sequences. They now pass alongside same-origin/default-port behavior, custom stop policies, redirect limits, and typed error propagation. Go 1.26.8 race tests, lint, and 91.8% coverage pass. Isolated Codex autoreview is scoped-clean at P0–P2.

Built-CLI proof uses two local synthetic HTTP servers:

Case	Before	After
Redirect to another origin, with a key echoed in the target URL	Destination receives the key; exit 0	Destination receives zero requests; exit 1 and redacted diagnostic
API error echoes the key	Key appears in stderr	[REDACTED]; HTTP status and exit 1 retained
Unknown flag / command contains ESC or a Unicode format control	Control bytes reach stderr	Controls removed; parser exit 80 retained
Normal details	Success	Success

The full 21-case built-CLI comparison for all eight commands remains byte-identical to baseline (requests, output, exit codes), SHA-256 043a9aa35c7e9a159a75b42afed4e4d56bf606646ee1352e3a22e8af3185eefe. All fixture values are synthetic; no live Google credentials are used.

[clawsweeper]

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

[clawsweeper]

Codex review: blocked before merge. Reviewed September 12, 2026, 11:09 PM ET / September 13, 2026, 03:09 UTC.

ClawSweeper review

What this changes

The PR restricts authenticated API redirects to the original origin, redacts echoed API keys while preserving error causes, and sanitizes CLI parser diagnostics.

Merge readiness

⛔ Blocked before merge - 1 item remains

Keep open: this is useful security hardening absent from current main and v0.4.9. No blocking correctness defect was found, and collaborator-authored work is protected from automatic closure.

Priority: P2
Reviewed head: 4698ccdfe3ecbae206104a01c44f43b74ded4acd

Review scores

Measure	Result	What it means
Overall readiness	🐚 platinum hermit (4/6)	A focused patch with relevant regression coverage, reported built-CLI comparisons, and a documented compatibility tradeoff.
Proof confidence	🌊 off-meta tidepool	Not applicable: The collaborator-authored PR is exempt from the external-contributor proof gate. Its captured body reports built-CLI rejection before cross-origin I/O, redacted errors, sanitized parser output, and baseline compatibility; source review found no unresolved authority case requiring additional final-effect proof.
Patch quality	🐚 platinum hermit (4/6)	No actionable review findings were identified.

Verification

Check	Result	Evidence
Real behavior	Not applicable	Not applicable: The collaborator-authored PR is exempt from the external-contributor proof gate. Its captured body reports built-CLI rejection before cross-origin I/O, redacted errors, sanitized parser output, and baseline compatibility; source review found no unresolved authority case requiring additional final-effect proof.
Evidence reviewed	7 items	Current main still needs the change: The pinned main client calls the supplied HTTP client directly and formats transport errors and API error bodies without key redaction. Main parser errors also bypass writeError. Release comparison: GitHub identifies v0.4.9 as the latest release. Its client also lacks the redirect restriction and diagnostic redaction, so the requested hardening has not already shipped there. Credential boundary and regression coverage: The redirect hook checks scheme, hostname, and effective port before follow-up I/O. Tests cover zero destination requests across origins, retained credentials within one origin, custom stop policies, redirect limits, redaction, and typed error causes. All eight API workflows use the shared request owner.
Findings	None	None.
Security	None	None.

How this fits together

goplaces exposes Google Places and Routes through a Go library and CLI. Its shared HTTP client sends authenticated requests, while the CLI turns parsing and API failures into terminal diagnostics.

flowchart TD
  A[CLI arguments] --> B[Argument parser]
  B --> C[Shared API client]
  D[Endpoint and API key] --> C
  C --> E{Redirect stays within origin?}
  E -->|Yes| F[API response]
  E -->|No| G[Redacted error]
  B -->|Invalid arguments| H[Sanitized terminal diagnostic]
  G --> H

Loading

Before merge

• Resolve merge risk (P1) - Existing endpoint overrides that rely on cross-origin redirects will stop working until configured to use the destination directly; the number of affected proxy setups is unknown.

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Production and test growth	Production +81/-11; tests +193/-0; documentation +4/-0	Production growth implements the stated credential boundary and error wrapper, with focused regression coverage.

Merge-risk options

Maintainer options:

1. Accept the documented redirect restriction (recommended)
   Retain the intentional security tightening and direct affected proxy users to configure the destination endpoint explicitly.

Technical review

Best possible solution:

Keep credentials scoped to the configured origin, with the documented direct-endpoint migration for proxies and sanitized diagnostics that retain inspectable error causes.

Do we have a high-confidence way to reproduce the issue?

Yes: a controlled redirect between two origins, an upstream error echoing the configured key, and invalid CLI arguments containing controls exercise the identified paths. Current-main source supports these mechanisms; this review did not execute them.

Is this the best way to solve the issue?

Yes: the shared request boundary and existing terminal sanitizer are the narrowest owners, and the documented security restriction is consistent with VISION.md.

AGENTS.md: not found in the target repository.

Codex review notes: model internal, reasoning medium; reviewed against d7a83610f74e.

Labels

Label changes:

• add P2: This is bounded security hardening with source-backed failure paths and no reported active exploitation or outage.
• add merge-risk: 🚨 compatibility: Previously accepted cross-origin redirects now fail, requiring direct endpoint configuration for affected proxy setups.
• add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
• add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The collaborator-authored PR is exempt from the external-contributor proof gate. Its captured body reports built-CLI rejection before cross-origin I/O, redacted errors, sanitized parser output, and baseline compatibility; source review found no unresolved authority case requiring additional final-effect proof.

Label justifications:

• P2: This is bounded security hardening with source-backed failure paths and no reported active exploitation or outage.
• merge-risk: 🚨 compatibility: Previously accepted cross-origin redirects now fail, requiring direct endpoint configuration for affected proxy setups.
• rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
• status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The collaborator-authored PR is exempt from the external-contributor proof gate. Its captured body reports built-CLI rejection before cross-origin I/O, redacted errors, sanitized parser output, and baseline compatibility; source review found no unresolved authority case requiring additional final-effect proof.

Evidence

What I checked:

• Current main still needs the change: The pinned main client calls the supplied HTTP client directly and formats transport errors and API error bodies without key redaction. Main parser errors also bypass writeError. (internal/places/client.go:104, d7a83610f74e)
• Release comparison: GitHub identifies v0.4.9 as the latest release. Its client also lacks the redirect restriction and diagnostic redaction, so the requested hardening has not already shipped there. (internal/places/client.go:104, 424df65ba142)
• Credential boundary and regression coverage: The redirect hook checks scheme, hostname, and effective port before follow-up I/O. Tests cover zero destination requests across origins, retained credentials within one origin, custom stop policies, redirect limits, redaction, and typed error causes. All eight API workflows use the shared request owner. (internal/places/client_security_test.go:18, 4698ccdfe3ec)
• Supplied behavior evidence: The complete captured PR body reports built-CLI before/after runs against two synthetic HTTP servers: cross-origin destination requests fall to zero, echoed keys become redacted, parser controls disappear, and ordinary details succeeds. It also reports 21 unchanged baseline cases across eight commands, with comparison SHA-256 043a9aa35c7e9a159a75b42afed4e4d56bf606646ee1352e3a22e8af3185eefe. These are contributor-reported observations; no separate transcript was supplied or executed by this review. (4698ccdfe3ec)
• Intentional compatibility tightening: VISION.md explicitly permits security fixes to reject previously accepted unsafe input. The PR documents direct endpoint overrides as the migration for cross-origin redirects. Photo media already requests skipHttpRedirect=true, so its documented URL-returning workflow does not require following a cross-origin redirect. (docs/client-reference.md:17, 4698ccdfe3ec)
• Historical routing: Main history associates Peter Steinberger with the recent client and CLI refactors; GitHub independently identifies steipete as author of merged refactor(client): separate route geometry and shared request helpers #50 and refactor(cli): organize commands and share request and rendering helpers #51. Older follow-history and blame traversal encountered unavailable blobs, so no source-line introduction claim is made. (internal/places/client.go, b9ba1ef76399)

Likely related people:

• steipete: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rating scale

Score	Internal tier	Crab rank	Meaning
6/6	S	🦀 challenger crab	Exceptional readiness
5/6	A	🦞 diamond lobster	Very strong readiness
4/6	B	🐚 platinum hermit	Good normal PR; ordinary maintainer review
3/6	C	🦐 gold shrimp	Useful, but confidence is limited
2/6	D	🦪 silver shellfish	Proof or implementation needs work
1/6	F	🧂 unranked krab	Not merge-ready
N/A	NA	🌊 off-meta tidepool	Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

• ClawSweeper keeps one durable marker-backed review comment per issue or PR.
• Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
• A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
• PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
• Maintainers can also comment @clawsweeper review to request a fresh review only.
• Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
• Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
• Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.