Skip to content

Harden client URL building against dot-segment path collapse (latent destructive-request retarget) #792

Description

@jiashuoz

Summary

Client code that interpolates an identifier into a URL path segment can be silently retargeted at a different endpoint. encodeURIComponent does not encode ., so a value of .. survives escaping, and the URL parser then resolves it as "go up one level" — before the request leaves the client. The server never sees the ..; it receives an ordinary-looking request to the parent resource.

Where the parent is itself a destructive endpoint, a routine delete becomes a much larger one, and it carries a valid confirmation token with it because ?confirm=DELETE is the same literal on every delete op.

This is latent, not live — see "Why nothing is broken today". Filing it because the only thing currently preventing it is a routing-library default that nothing in the codebase asserts or tests.

Mechanism

encodeURIComponent("..")                    -> ".."            (dots are unreserved)
new URL("/v1/account/suppressions/..").pathname -> "/v1/account/"

Since .. also consumes the segment before it, every two-segment delete resolves one level up. Mapping the collapse target of each DELETE route against the set of DELETE routes:

Route Collapses to Parent is a DELETE endpoint?
/v1/account/suppressions/{address} /v1/account yes — deletes the account
/v1/account/api-keys/{id} /v1/account yes — deletes the account
/v1/agents/{email}/suppressions/{address} /v1/agents/{email} yes — deletes the agent
/v1/agents/{email}/contacts/{address} /v1/agents/{email} yes — deletes the agent
/v1/contacts/{address}, /v1/domains/{domain}, /v1/templates/{id}, /v1/webhooks/{id} /v1 no
/v1/contacts/imports/{batch_id} /v1/contacts no

The confirmation token does not help: DeleteConfirm (internal/httpapi/delete_confirm.go) is one shared type with enum:"DELETE" used by all 12 delete ops, so a token minted to authorize "remove one suppression" is byte-identical to one authorizing "delete this account". The caller's own safety check satisfies the escalated operation's guard.

Why nothing is broken today

Two independent accidents, both verified against a local server:

  1. Values can't reach the list. handleCreateAgentSuppression validates with mail.ParseAddress, so no stored address is all-dots. Same for contacts.
  2. chi does not redirect trailing slashes. DELETE /v1/account/?confirm=DELETE404, account intact. DELETE /v1/agents/{email}/?confirm=DELETE404, agent intact.

(2) is the load-bearing defense and it is a library default — nothing in the repo asserts it. A chi upgrade or a StrictSlash-style config change would make every row in the table above live simultaneously, with no test failing.

Scale of the exposure

  • 64 encodeURIComponent-into-path call sites across 11 files in sdks/typescript/src/v1/generated/apis/ — all regenerated by OpenAPI Generator, so per-call-site patches are erased by the next make generate.
  • Web dashboard pages build paths with raw fetch and are not covered by the SDK.
  • PR feat: suppressions across web, CLI, and MCP surfaces #791 added client-side guards for the two suppression paths only (web/src/app/(app)/contacts/_lib/suppressionPath.ts, checkedAddress() in cli/src/commands/suppressions.ts). The api-keys and outreach-contacts paths are unguarded.

Proposed fixes, in priority order

  1. SDK middleware guard (covers CLI + MCP + all SDK users at once). Every request passes through the hand-written pre(context) hook in sdks/typescript/src/v1/generated/middleware.ts's middleware chain after the path is built. Reject any outgoing request whose path contains a ./.. segment. Lives outside generated call sites, so make generate can't erase it. Mirror in the Python SDK.
  2. Router regression test pinning that the trailing-slash form of every destructive route 404s. Converts today's accidental defense into an asserted guarantee, and is the thing that catches a future chi upgrade.
  3. Web helper rollout — apply the encodeAddressSegment pattern from feat: suppressions across web, CLI, and MCP surfaces #791 to the remaining raw-fetch pages (api-keys, outreach, contacts, domains, templates, webhooks).
  4. (Design discussion, wire change) Scale confirmation to blast radius: have the highest-consequence deletes name their target (e.g. account deletion requiring the account's own address, the way GitHub requires typing a repo name) instead of a universal DELETE literal. This is the only option that fails closed regardless of routing behavior or client correctness — a misrouted request carrying confirm=DELETE would be rejected on arrival.

1 and 2 together are small and remove the dependency on a library default; they are the recommended first PR.

Repro

node -e 'console.log(new URL("/v1/account/suppressions/..", "https://api.e2a.dev").pathname)'
# -> /v1/account/

Before PR #791's guard, e2a suppressions remove '..' printed deleted .. and exited 0 while the server received DELETE /v1/account/?confirm=DELETE.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingsecuritySecurity-related issue or hardening

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions