security: reject foreign-host URLs in api to prevent token leak

[jeremy]

Reject foreign-host URLs in api to prevent bearer-token leak

Severity: High — credential exfiltration primitive. (Threat: T1 malicious/compromised API target.)

parsePath only extracted the path from absolute URLs whose first segment was a numeric account ID. Any other absolute URL (e.g. https://evil.example/projects.json) failed the regex, fell through the no-op TrimPrefix, and was returned verbatim to app.Account().Get/Post/Put/Delete — the SDK then attached the Authorization: Bearer header and sent the request to the foreign host.

Fix

parsePath now returns (string, error) and rejects absolute http(s):// URLs that aren't the canonical https://<host>/<numeric-account-id>/... form, before any network call. Error propagated through all four api handlers.

Tests

api_test.go adds a table for relative / leading-slash / valid-account / foreign-host inputs (TestParsePathRejectsForeignHosts). Legitimate https://3.basecampapi.com/999/... still extracts correctly.

---

Part 1/6 of a stacked security-hardening series (audit remediation). This one is independent and safe to land alone; the rest stack on top in order. Base: main.

📚 Stack (merge bottom-up)

1. security: reject foreign-host URLs in api to prevent token leak #478 — reject foreign-host URLs in api (base main)
2. security: strip ANSI/OSC escapes from API-controlled output #479 — strip ANSI/OSC escapes from output
3. security: harden OAuth discovery and token endpoints #480 — harden OAuth discovery / token endpoints
4. security: close config trust-boundary gaps and gate completion loader #481 — close config trust-boundary gaps + completion gate
5. security: tighten config-dir perms and validate plugin scope argv #482 — tighten config-dir perms + plugin scope argv
6. security: bump toolchain/x-net for CVEs and tighten CI gates #483 — bump toolchain/x-net + CI gates

Each is independent except #482 depends on #481 (shared root.go). #478 can land first/alone.

[github-actions]

⚠️ Potential breaking changes detected:

• The function parsePath was modified to reject absolute URLs pointing to foreign hosts. URLs that were previously accepted by parsePath, even if they were on foreign hosts, will now cause an error, which can change the behavior of the CLI and break existing scripts.

Review carefully before merging. Consider a major version bump.

[Copilot]

Pull request overview

Hardens the basecamp api subcommand against a credential-exfiltration vector. Previously, parsePath silently passed any non-canonical absolute URL through to the SDK, which would then attach the user's Authorization: Bearer header and send it to the foreign host. parsePath now returns an error for absolute http(s):// URLs that aren't of the form https://<host>/<numeric-account-id>/..., and all four api verb handlers propagate that error before any network call.

Changes:

• Change parsePath signature from (string) to (string, error) and reject absolute URLs that don't match the canonical Basecamp account-id form.
• Wire error propagation through the get/post/put/delete RunE handlers.
• Add TestParsePathRejectsForeignHosts and update TestParsePath for the new return signature.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
internal/commands/api.go	parsePath now returns an error and rejects non-canonical absolute URLs; all four handlers propagate the error.
internal/commands/api_test.go	Updates existing test to new signature; adds table-driven test asserting rejection of foreign/non-canonical URLs.

Tip

If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[cubic-dev-ai]

2 issues found across 2 files

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2eacce7710

ℹ️ 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".

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

[robzolkos]

The accepted-behavior here looks intentional rather than a gap: this PR added the assertion together with an explanatory comment — // Same host but no account segment — accepted, path used as-is. — so the description is the stale part, not the code.

The contract in the description (https://<host>/<numeric-account-id>/...) describes the original shape-based design. Over the course of review this became a host-based check (EqualFold(u.Host, base.Host)), which relocated the security property from 'does it have the account-id shape?' to 'is the host the one we're authenticated against?'. The account segment is irrelevant to the token-leak goal — only the host determines where the credential is sent.

Keeping the lenient acceptance is also the more consistent choice: api get projects.json and api get https://<configured-host>/projects.json are the same effective request (same host, path /projects.json, SDK re-prefixes the account). Rejecting the second while accepting the first would be arbitrary, and tightening the code would reject a harmless, equivalent input.

Recommend resolving this by updating the description to 'relative paths, or absolute URLs on the configured Basecamp host (foreign hosts rejected)' rather than changing the code. Leaving this for @jeremy to make the call.