Pin SDK bearer token to the configured origin (all 6 SDKs)#326
Open
jeremy wants to merge 15 commits into
Open
Conversation
buildURL now rejects an absolute URL whose origin differs from the configured base URL, and a new assertCredentialOrigin backstop runs at both token-attach sites (singleRequest and the generated client's authEditor) so a foreign-origin URL passed as a request path can never carry the Authorization header off-origin. Localhost is carved out for dev/httptest. Mirrors the same-origin guards already used for pagination Link headers and cross-origin redirects.
Add requireSameOrigin / isSameOriginAllowingLocalhost / requireSecureEndpoint to security.ts and apply them in createAuthMiddleware, fetchPage, and authenticatedFetch so the Authorization header is attached only to the configured origin (localhost carve-out). HTTPS-validate the authorization.json endpoint. The intentional cross-origin Launchpad authorization call is preserved.
build_url rejects foreign-origin absolute URLs and assert_credential_origin! backstops both single_request / single_request_raw paths so credentials are attached only to the configured origin (localhost carve-out). get_absolute gains an allow_cross_origin flag (HTTPS-validated) for the intentional cross-origin Launchpad call.
_build_url rejects foreign-origin absolute URLs and a _single_request backstop enforces same-origin credential attach in both the sync (_http.py) and async (_async_http.py) clients (localhost carve-out). get_absolute gains an allow_cross_origin flag for the intentional cross-origin Launchpad call.
BaseService.accountUrl rejects foreign-origin absolute URLs and BasecampHttpClient.requireSameOrigin backstops request/requestBinary so credentials are attached only to the configured origin (localhost carve-out; isLocalhost promoted to internal in Pagination.kt and deduped from BasecampClient.kt). The retry wrappers now rethrow a BasecampException from the guard instead of masking it as a retryable Network error, so a foreign-origin request surfaces as BasecampException.Usage.
BaseService.buildURL rejects foreign-origin absolute URLs and HTTPClient.assertSameOrigin backstops performRequest/fetchPage so the Authorization header is attached only to the configured origin (localhost carve-out via Pagination.swift isLocalhost).
Add a Same-Origin Credential Attachment subsection under Credential Protection describing the two-layer guard (URL-build chokepoint + token-attach backstop) enforced across all six SDKs.
github-actions
Bot
added
documentation
github-actions
Bot
added
bug
![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: 7feef8c941
ℹ️ 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".
There was a problem hiding this comment.
Pull request overview
This PR hardens all SDKs by preventing bearer token leakage when callers pass absolute URLs: credentials are now only attached to requests targeting the configured base-URL origin (with a localhost dev/test carve-out), with a second “backstop” check right before attaching Authorization.
Changes:
- Add same-origin validation at URL build time and re-check at credential-attach time across Go/TypeScript/Ruby/Python/Kotlin/Swift.
- Preserve the intentional cross-origin Launchpad authorization call by explicitly allowing it (while still validating endpoint security where applicable).
- Add same-origin regression tests in each SDK and document the invariant in
SECURITY.md.
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.
Reviewed changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| typescript/tests/services/authorization.test.ts | Adds endpoint-override validation tests for authorization info retrieval. |
| typescript/tests/security.test.ts | Adds unit coverage for same-origin guard helpers and fail-closed behavior. |
| typescript/src/services/authorization.ts | Validates custom authorization endpoint overrides as secure before attaching token. |
| typescript/src/security.ts | Introduces same-origin/secure-endpoint guard utilities used by the client. |
| typescript/src/client.ts | Enforces same-origin before auth attachment (middleware + raw fetch helpers). |
| swift/Tests/BasecampTests/SameOriginGuardTests.swift | Adds Swift regression tests ensuring no token egress to foreign origins. |
| swift/Sources/Basecamp/Services/BaseService.swift | Routes URL building through a chokepoint that rejects foreign-origin absolute URLs. |
| swift/Sources/Basecamp/Pagination.swift | Adds isLocalhost helper to support localhost carve-out consistently. |
| swift/Sources/Basecamp/HTTP/HTTPClient.swift | Adds attach-point backstop to prevent adding auth on foreign-origin requests. |
| SECURITY.md | Documents the same-origin credential attachment invariant and the two enforcement layers. |
| ruby/test/basecamp/security_test.rb | Updates Ruby security tests to assert cross-origin absolute URLs are rejected. |
| ruby/test/basecamp/same_origin_test.rb | Adds Ruby end-to-end same-origin credential attachment tests (incl. Launchpad exception). |
| ruby/test/basecamp/http_test.rb | Updates Ruby HTTP tests to reject cross-origin absolute URLs without egress. |
| ruby/lib/basecamp/http.rb | Implements build-time guard + attach-time backstop; adds explicit cross-origin allowance for Launchpad. |
| python/tests/test_http.py | Adds Python regression tests ensuring foreign-origin absolute URLs fail without egress. |
| python/src/basecamp/_http.py | Adds build-time same-origin enforcement and attach-time backstop (sync client). |
| python/src/basecamp/_async_http.py | Adds build-time same-origin enforcement and attach-time backstop (async client). |
| kotlin/sdk/src/commonTest/kotlin/com/basecamp/sdk/SameOriginTest.kt | Adds Kotlin tests for same-origin enforcement and token attachment behavior. |
| kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/services/BaseService.kt | Adds URL-build chokepoint guard for absolute URLs in service URL construction. |
| kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/Pagination.kt | Promotes/deduplicates isLocalhost helper for reuse in guards. |
| kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/http/BasecampHttpClient.kt | Adds attach-point backstop + preserves exception classification through retry wrappers. |
| kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/BasecampClient.kt | Removes duplicated localhost helper after moving it to Pagination.kt. |
| go/pkg/basecamp/same_origin_test.go | Adds Go end-to-end tests for same-origin credential attachment behavior. |
| go/pkg/basecamp/client.go | Adds same-origin enforcement in buildURL and an attach-point backstop in request auth sites. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
8 issues found across 24 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
The same-origin guard accepted any absolute HTTPS URL whenever the configured base URL was localhost, so a localhost-base client (a common local-dev config) would attach the bearer token to a foreign origin — defeating the guard. Drop the base-localhost clause at both guard sites (buildURL and assertCredentialOrigin) so the carve-out keys off the target, matching every other SDK.
github-actions
Bot
added
documentation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fb12b4a9b8
ℹ️ 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".
Follow-up to the same-origin credential guard, addressing review findings on PR #326: Python (P1): get_absolute() passed allow_cross_origin=True unconditionally, a blanket bypass reachable via the public client.http that would attach the bearer token to any foreign origin. Scope the allowance to the one trusted cross-origin destination — the Launchpad authorization endpoint — and reject every other foreign origin. The canonical URL now lives in _security. Kotlin: accountUrl's absolute-URL detection was case-sensitive, so an uppercase-scheme URL was misclassified and mangled into an account path; make scheme detection case-insensitive and normalize origin case in extractOrigin so same-origin comparison holds. isLocalhost did not recognize the bracketed IPv6 loopback [::1]; parse the bracketed authority. TypeScript: isLocalhost received bracketed IPv6 hostnames from URL.hostname (e.g. "[::1]") and failed the loopback match; strip the brackets, fixing the carve-out for every caller (same-origin guard and endpoint validation). Adds regression tests in each SDK. Go suite, Python (218), Kotlin jvmTest, and TypeScript (628) all green.
github-actions
Bot
added
the
documentation
github-actions
Bot
added
breaking
Ruby's Http#get_absolute passed allow_cross_origin: true unconditionally,
bypassing both build_url and the assert_credential_origin! backstop. Since
client.http is public, client.http.get_absolute("https://evil.example/x")
would attach the bearer token to any foreign origin — the same blanket-bypass
flagged and fixed in the Python SDK.
Scope the allowance to the trusted Launchpad authorization endpoint; every
other foreign origin trips the same-origin guard before egress. The canonical
URL now lives in Security and is single-sourced by AuthorizationService so the
guard and the discovery fallback can't drift apart. The OAuth-discovery path
resolves to the same Launchpad issuer, so it is unaffected. Regression test
added; full Ruby suite (609) green.
github-actions
Bot
added
documentation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b1aa2a932a
ℹ️ 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".
Round of PR #326 review feedback across SDKs: Swift (BaseService.buildURL): allowed plain HTTP only for localhost targets (rejecting every other http:// as before) and made scheme detection case-insensitive, so http://localhost dev URLs and uppercase schemes are handled correctly. isSameOrigin now compares scheme/host case-insensitively (RFC 3986) so the carve-out holds for mixed-case URLs. Regression tests added. Kotlin (isLocalhost): compare the extracted host case-insensitively. Python (_http/_async_http): truncate caller-supplied URLs in the same-origin UsageError messages via _security.truncate, matching the SDK's other URL-bearing errors and bounding error/log size. Ruby (http.rb): clarify the assert_credential_origin! comment — allow_cross_origin is granted only by get_absolute and only for the trusted Launchpad endpoint; a discovery-provided foreign issuer is not bypassed. SECURITY.md: localhost definition now lists .localhost and bracketed [::1] and notes case-insensitive matching; the cross-origin exception is described as the authorization endpoint (default Launchpad, overridable in some SDKs). Python (218), Kotlin jvmTest, and Ruby suites green; Swift edits parse and type-check (CI builds on macOS).
github-actions
Bot
added
the
documentation
github-actions
Bot
removed
the
documentation
AGENTS.md forbids editing files under */generated/ (regeneration overwrites them). Revert the authorization_service.rb constant to its generated literal and keep Security::LAUNCHPAD_AUTHORIZATION_URL as the non-generated single source that Http#get_absolute scopes against. A new test asserts the two stay equal, so a future regeneration that drifts the generated literal is caught by CI before it can break the scoped Launchpad cross-origin allowance.
github-actions
Bot
added
documentation
Kotlin's isLocalhost matched only localhost/127.0.0.1/::1, so *.localhost dev/test hosts (e.g. myapp.localhost) were rejected — diverging from the other SDKs and from the localhost definition in SECURITY.md. This helper backs base-URL HTTPS validation and both same-origin guards, so the gap affected all three. Add the .localhost suffix check and tests (including lookalike negatives).
github-actions
Bot
added
documentation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f15eaa8ef3
ℹ️ 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".
The exact-match Launchpad allowance broke a non-Launchpad issuer resolved by OAuth discovery (e.g. a staging/custom auth server): its authorization endpoint hit assert_credential_origin! and failed instead of fetching authorization info. Scope get_absolute's cross-origin allowance to the authorization-endpoint shape (an HTTPS .../authorization.json URL) via Security.authorization_endpoint?, which the discovery flow always produces — so any discovered issuer works while every other foreign origin (e.g. https://evil.example/x) still trips the same-origin guard. This also drops the Security constant that was coupled to the generated fallback, so no generated code is involved. Regression test added for a discovered non-Launchpad issuer; full Ruby suite (610) green.
github-actions
Bot
added
the
documentation
github-actions
Bot
removed
the
documentation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 973ff76051
ℹ️ 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".
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
The path-based allowance (any HTTPS .../authorization.json) was too loose: it
let client.http.get_absolute("https://evil.example/authorization.json") attach
the bearer token to a foreign host (cubic P0, Codex P2). Restrict the bypass to
the exact Launchpad authorization URL again. For the Basecamp SDK, OAuth
discovery resolves to the Launchpad issuer, so the authorization flow is
unaffected; a discovered same-origin issuer still works via the same-origin
guard. Supporting an arbitrary different-origin discovered issuer cross-origin
can't be distinguished from the exfiltration case without reintroducing the
leak, so it is intentionally not granted the bypass.
Also fix build_url to allow plain HTTP for localhost targets (matching
require_https_unless_localhost! and the dev/test carve-out) and truncate the
HTTPS-required error message, per the build_url review.
Regression tests: foreign /authorization.json-shaped URL rejected; localhost
http:// accepted; Launchpad constant stays in lockstep. Full Ruby suite (612) green.
github-actions
Bot
added
documentation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6e1ae8db8c
ℹ️ 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".
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Same-origin credential attachment (all 6 SDKs)
Hardening: enforce the invariant that the bearer /
Authorizationtoken is attached only to the configured origin (with a localhost carve-out for dev/test) across every SDK.Why
Clients accept an absolute URL in the request "path" position (used legitimately for pagination
Linkheaders and similar). The URL-building code previously checked only the scheme (HTTPS vs HTTP), never the origin. So a caller that passed an absolute foreign URL — e.g.client.Get(ctx, "https://evil.example/x.json")— would have the bearer token attached and sent to a foreign host: a credential-exfiltration primitive (High severity, not known exploited).This mirrors the same-origin guards already applied to pagination
Linkheaders and cross-origin redirects, and the existing base-URL HTTPS check.What
Two layers per SDK, failing closed:
buildURLassertCredentialOriginatsingleRequest+ the generated client'sauthEditorrequireSameOrigin/requireSecureEndpointinsecurity.tscreateAuthMiddleware,fetchPage,authenticatedFetchbuild_urlassert_credential_origin!atsingle_request/single_request_raw_build_url_single_request(sync and async clients)BaseService.accountUrlBasecampHttpClient.requireSameOrigininrequest/requestBinaryBaseService.buildURLHTTPClient.assertSameOrigininperformRequest/fetchPageNotes:
authorization.jsoncall is explicitly preserved (Ruby/Python via anallow_cross_originflag that still HTTPS-validates the endpoint).BasecampException.Usageinstead of masking it as a retryableNetworkerror;isLocalhostwas promoted tointernaland deduplicated.spec/or*/generated/edits.Tests
End-to-end invariant asserted in every SDK: a request to a foreign-origin absolute URL errors before any network send, and the mock transport records no
Authorizationegress; same-origin and localhost absolute URLs still carry the token; same-origin pagination and the OAuth/Launchpad call still work.New suites:
go/.../same_origin_test.go,typescript/tests/security.test.ts(+services/authorization.test.ts),ruby/.../same_origin_test.rb, PythonTestSameOriginGuard,kotlin/.../SameOriginTest.kt,swift/.../SameOriginGuardTests.swift.Verification
make -C go test— green (same-origin suite passes).npm test— 625 passed.rake test— 608 runs, 0 failures.uv run pytest— 217 passed../gradlew :basecamp-sdk:jvmTest— 181 passed, 0 failures (incl. the new suite). The aggregatetest/:generatortasks need a JDK ≤ 25 toolchain; the SDK module's own JVM tests run on JDK 26.CFAbsoluteTime/FoundationNetworkinggaps in untouched code); the test follows the verifiedMockTransport/makeTestAccountClientpatterns.Disclosure
Please coordinate disclosure via a private GitHub Security Advisory or
security@basecamp.comrather than a public issue.Summary by cubic
Pins the bearer Authorization token to the configured base-URL origin across Go, TypeScript, Ruby, Python, Kotlin, and Swift, with a localhost carve-out. Scopes the only cross-origin credentialed call to the exact Launchpad authorization endpoint.
Bug Fixes
requireSecureEndpoint.https://launchpad.37signals.com/authorization.json.[::1], adds RFC 6761.localhost, and surfaces guard errors asBasecampException.Usage.isLocalhostfor bracketed IPv6 and addsrequireSameOrigin.SECURITY.mddefines localhost (localhost, any*.localhost,127.0.0.1,::1/[::1], case-insensitive); new tests assert foreign-origin URLs fail before any network send and never includeAuthorization.Migration
Written for commit 6e1ae8d. Summary will update on new commits.