Authenticate CLI API requests

[raghubetina]

Summary

• require FIRSTDRAFT_API_TOKEN for plan push, plan status, and plan compile
• send the Bearer credential on every API request, including compilation polls and artifact download
• expose missing credentials and the validated First Draft authentication_required problem through one stable CLI error
• keep tokens out of .firstdraft, command output, and local-only commands
• document the credential and recovery contract

Contract

A missing or whitespace-only token stops before local input or network access. Invalid and revoked credentials are validated by First Draft; only its validated 401 problem with code authentication_required is promoted to the stable authentication error. Other unvalidated responses retain the CLI's conservative protocol and mutation-recovery handling.

Verification

• npm run check
  • TypeScript check
  • ESLint
  • Prettier
  • 116 tests
  • npm package allowlist
  • installed-tarball smoke, including authenticated compilation start and artifact download
• git diff --check

[raghubetina]

Technical review

A PR that introduces a bearer credential is worth attacking rather than reading, so I went looking for ways the token could end up somewhere it should not. It cannot, and the reasons are worth writing down because most of them are deliberate rather than incidental.

npm run check exits 0 with 116 tests passing, matching the body.

The one that would have been serious

downloadArtifact builds its endpoint from a value the server supplied:

const endpoint = new URL(metadata.path, apiUrl);

new URL(relative, base) ignores the base entirely when the first argument is absolute, so a compromised or hostile First Draft could return artifact.path = "https://attacker.example/collect" and the authenticated fetch would deliver the bearer token there.

That is closed properly, in isCompilationPath:

typeof value !== "string" ||
!value.startsWith("/") ||     // absolute URLs
value.startsWith("//") ||     // protocol-relative
value.includes("\\")          // backslash tricks

const parsed = new URL(value, "https://firstdraft.invalid");
parsed.origin === "https://firstdraft.invalid" &&
parsed.pathname === value &&      // no encoding or traversal
parsed.search === "" && parsed.hash === "" &&
(value === scope || value.startsWith(`${scope}/`))

Rejecting the absolute form, the protocol-relative form, and anything whose parsed pathname differs from the input covers the escapes. Requiring the value to sit inside /v1/projects/{id}/compilations/{id} means even a valid relative path cannot wander to another project's resources. status_path goes through the same check, and the artifact request additionally sets redirect: "error", so the redirect route is closed too.

Where else the token could go

The destination origin is constrained before any of this, in normalizeApiUrl:

const loopbackHttp = url.protocol === "http:" &&
  (url.hostname === "127.0.0.1" || url.hostname === "localhost" || url.hostname === "[::1]");
if ((url.protocol !== "https:" && !loopbackHttp) || url.username !== "" || url.password !== "" || ...)

HTTPS anywhere, plaintext HTTP only to the three loopback names, and embedded credentials rejected so https://user:pass@host cannot smuggle anything. This is the same policy the iOS Core applies to APP_ROOT_URL, which is a good sign that it is a considered position rather than a local accident.

Behaviour I checked rather than assumed

undefined token  -> null
whitespace token -> null
empty token      -> null
call 1 headers: {"Accept":"application/json","Authorization":"Bearer tok123"}
call 2 headers: {"Authorization":"Bearer tok123"}

The second call passed Authorization: Bearer attacker in init.headers, and the real token replaced it rather than being appended or displaced. That ordering matters: spreading ...init?.headers before setting Authorization is what makes the credential authoritative, and it would be a silent hole if the two lines were swapped.

Returning null for a blank token, with each command treating null as a hard stop before local input or network access, is the right shape for a fail-closed check.

The remaining claims hold

No reference to a token anywhere in plan-state.js or file-system.js, so nothing persists it to .firstdraft. plan init is not wired to apiToken, so the local-only command stays usable without credentials. writeAuthenticationRequired emits error, detail, and the server's own problem document, with no path that could echo the credential.

isAuthenticationProblem requiring all three of HTTP 401, response.status === 401, and code === "authentication_required" is appropriately narrow. A bare 401 from a proxy or a misconfigured gateway keeps the conservative protocol handling instead of being promoted to a stable authentication error that tells users to go fix their token.

No findings

[raghubetina]

Lesson: never send a credential to a URL somebody else chose

Here is a line that appears in this PR, and something very like it appears in most API clients:

const endpoint = new URL(metadata.path, apiUrl);

metadata.path came from the server's response. apiUrl is your own trusted origin. The intent is obvious: take the relative path the API gave us and resolve it against the API's base.

Now the part that surprises people. If the first argument is an absolute URL, the base is ignored completely.

new URL("/v1/artifacts/42", "https://api.example.com")
// => https://api.example.com/v1/artifacts/42     as intended

new URL("https://attacker.example/x", "https://api.example.com")
// => https://attacker.example/x                  base discarded

This is correct behaviour, specified, and identical in every language's URL resolver. It is also a credential-exfiltration primitive, because the next line attaches your bearer token and calls it.

The general rule

A URL from a response is untrusted input, exactly like a form field.

We internalised that user input needs validation. Server responses feel different, because we think of "the server" as ours. But the value only has to become attacker-controlled once, through a compromised dependency, a misconfigured proxy, a hostile registry, or a bug that lets one tenant write another tenant's record, and the client will faithfully carry credentials wherever it is pointed.

Three variants worth recognising:

Absolute URL where a path was expected, as above.

Protocol-relative URLs. //attacker.example/x looks like a path and is not. It inherits the current scheme and replaces the host. This PR rejects it explicitly, which is why the check tests startsWith("//") separately from startsWith("/").

Redirects. Even a valid relative path can respond 302 to somewhere else. The Fetch specification strips Authorization on cross-origin redirects, which helps, and the code here does not rely on it: the artifact request sets redirect: "error".

What this looks like in Rails

response = HTTParty.get(payload["next_url"], headers: { "Authorization" => "Bearer #{token}" })

Same shape. next_url came from a response body and the token goes wherever it points.

The variants you will actually meet:

redirect_to params[:return_to]           # open redirect; use redirect_to ... allow_other_host: false
Net::HTTP.get(URI(user.avatar_url))      # SSRF; can reach 169.254.169.254 and your cloud metadata
send_webhook(to: integration.callback_url, headers: auth_headers)   # tenant-supplied destination

Rails 7 made redirect_to refuse external hosts by default precisely because this class of bug kept recurring.

The defence that actually works

Validate the shape, not the badness. Blocklists lose.

The check in this PR is worth reading as a template, because it establishes four separate things:

!value.startsWith("/") || value.startsWith("//") || value.includes("\\")

then re-parses and confirms the result did not escape:

parsed.origin === "https://firstdraft.invalid" &&
parsed.pathname === value &&
parsed.search === "" && parsed.hash === ""

and finally requires the path to sit inside the specific resource it belongs to.

Parse it, then check that parsing did not change it. parsed.pathname === value is the clever part: it catches percent-encoding, dot-segments, and anything else that normalises to something other than what you were handed.

The Ruby equivalent for the common case:

def safe_callback_uri(candidate)
  uri = URI.parse(candidate)
  raise ArgumentError unless uri.is_a?(URI::HTTPS)
  raise ArgumentError unless ALLOWED_HOSTS.include?(uri.host)
  raise ArgumentError unless uri.userinfo.nil?
  uri
end

An allowlist of hosts, a required scheme, and a rejection of embedded credentials. Three lines, and it removes an entire category.

The habit

When you attach a credential to an outbound request, ask one question: who decided this URL?

If the answer is anything other than "my own configuration, validated at load," the credential should not go until the destination has been checked. And if the destination is genuinely dynamic, the right move is usually a scoped token that is worthless elsewhere, rather than a better validator.