basecamp / basecamp/basecamp-sdk
Ruby: enforce the OAuth address policy by pinning resolved addresses in the default Fetcher transport (SPEC §16 req 5–6)
- Dominant language
- Go
- Stars
- 49
- Forks
- 12
- Avg merge
- 20h 47m
- Merged PRs (30d)
- 89
Description
Extend SPEC §16 requirements 5–6 (dial-time address judgement of the advertised issuer, `token_endpoint`, and `device_authorization_endpoint`) to the Ruby SDK. Appendix F's tables record today's state: scheme gate, bounded timeout, suppressed redirects, bounded body — an advertised issuer or discovered endpoint naming a private address is still dialed.
**Step 0 (upstream, blocking):** the surfguard gem's 0.2.0 release — RubyGems still serves 0.1.3, which predates the `policy:` argument, the IPv6-allocated allowlist, and the conformance corpus. Tracked as basecamp/surfguard#24.
**Enforcement design** (the gem is classification/resolve-only by design; enforcement is ours):
- Use the gem README's tested `Net::HTTP` pinning recipe (`test/readme_recipe_test.rb` executes it from the README): `Surfguard.resolve_public_ips(host, policy: ...)` then pin via `ipaddr=`, keeping the hostname for SNI/Host, with the proxy disabled on the pinned connection.
- Wire it into `Oauth::Fetcher.stream_http` — the default transport for both discovery hops, the device flow, the exchange path, and the legacy provider — so every SDK-built OAuth request gets the same judgement.
- **Connect-time DNS-rebinding resistance is mandatory:** the address judged MUST be the address connected. The pin (resolve once, connect to exactly that address) satisfies this; re-pin on every hop if any hop is ever added. A preflight classification followed by Net::HTTP's own second resolution is a rebinding window, not an implementation.
- **The injected-Faraday lane cannot pin** and stays caller-owned: a caller-supplied `http_client:` is the caller's, enforcement included — the same contract as Go's injected clients (SPEC §16). Document it in the constructor docs.
- **Loopback is explicit operator configuration.** The gem deliberately ships no loopback mode. Admission MUST be a policy option on the SDK constructors (mirroring Go's `AllowLoopback` derivation and `WithIssuerPolicy`/`WithDevicePolicy`/`WithExchangerPolicy` overrides) — never inferred from the metadata-provided endpoint or any request-derived value, which would recreate the SSRF hole. The existing localhost HTTPS exemption is a *scheme* judgement and does not admit loopback addresses through this policy.
- **Overrides (SPEC §16 contract):** replacement policy, replacement client, and disablement (an injected client counts).
- **Verdicts:** refusal is `invalid_issuer_origin` on the discovery hop, `api_error` elsewhere, non-retryable; the device poll terminates on first refusal. Surfguard's unresolvable-vs-blocked distinction survives into the SDK's taxonomy (unresolvable stays a retryable network fault).
**Definition of done:** Appendix F's requirement-5/6 rows for Ruby show enforcement; per-SDK unit tests assert refusal-before-dial (no connection attempted) for private/loopback/CGNAT/special-purpose targets on all three request kinds, plus the override and injected-client contracts.
Contributor guide
Research direction
Begin with Oauth::Fetcher.stream_http and the README's tested Net::HTTP pinning recipe, exercised by test/readme_recipe_test.rb; confirm the surfguard 0.2.0 dependency is available first. Trace the discovery, device, exchange, and legacy-provider paths, then add the required refusal, override, and injected-client tests. Done means Appendix F requirement-5/6 rows show enforcement and refusal occurs before dialing for the listed target classes.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- ruby
- Domain
- api, security
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100