Use the omitted-window contract for all-time activity so mixed-version restarts do not reject the page, and remove the obsolete audit footer.
Co-Authored-By: Paperclip <noreply@paperclip.ing>
Read the cross-app activity feed from the canonical tool call events used by each connection, preserving filters, pagination, gateway scope, and redacted invocation details.
Co-Authored-By: Paperclip <noreply@paperclip.ing>
Backport the existing master guard so this long-lived feature worktree does not fan out Git scans across its cloned workspace inventory.
Co-Authored-By: Paperclip <noreply@paperclip.ing>
Removing an app only flipped the connection to `archived`. Everything that
made it work stayed behind: the operator's pasted key or OAuth tokens as live
secrets, the `company_secret_bindings` that project them into runtimes, the
install, the app-managed `app:<connectionId>` profile, gateway tokens minted
against that profile, and any authorization still in flight. Reconnecting
reused the retained credential, so "remove" never actually revoked anything.
Removal is now a teardown, and the order is the security property. Every
database-side access path closes first — grants revoked, bindings and installs
deleted, the app profile removed (archived instead when a gateway's RESTRICT
foreign key forbids deleting it, with that gateway's tokens and sessions
revoked), live local runtimes stopped, catalog entries marked `removed`,
outstanding OAuth state discarded, the connection and its application archived
— so the app is undispatchable before the first call out to a secret provider.
Only then are the credentials revoked, and each secret's ref survives until
that secret is gone: a provider that errors leaves the operation failed closed
and resumable, because the secret row is marked deleted before the provider
call and the surviving refs are what a retry reads to finish the job.
A secret is destroyed only when two independent tests agree: it was minted
into the `tool_app.` namespace by connect/reconnect/OAuth, and nothing outside
this connection references it. Anything ambiguous keeps its row and is
reported as retained — deleting a credential another target still resolves is
an outage, retaining an unreachable row is not.
What stays is deliberate. Connection and application identity, names and
activity survive so a reconnect reuses them, but with no credential, install
or profile, reconnecting has to ask for fresh authentication and rebuild
access. The activity receipt carries cleanup counts only; six of those count
keys name what they counted ("secrets", "token"), so `sanitizeRecord` lets
them through while the value is a finite number and still blanks the same key
carrying anything else.
Refs PAP-17119, PAP-17118, PAP-17089 (QA finding F6).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Map OAuth origin, duplicate-name, and DNS failures to stable operator guidance, and derive collision-resistant defaults from host, port, and path.
Co-Authored-By: Paperclip <noreply@paperclip.ing>
Prevent connection installs from adding connection-wide allow entries, keep wizard-managed app profiles additive across binding precedence, and repair legacy over-grants with a data migration. Add decision-level regression coverage for generic and curated MCP paths.
Co-Authored-By: Paperclip <noreply@paperclip.ing>
Preserve the unverified-server trust marker on URL-only connection details and compact the mobile action step above the bottom nav.
Co-Authored-By: Paperclip <noreply@paperclip.ing>
A denial or a cancel is a final answer to an authorization request, but
`completeOAuthCallback` treated it as merely a failed callback: the row
in `tool_oauth_states` survived, so the `state` the user just refused
stayed completable for the rest of its 10-minute TTL by anyone who could
produce a code. Any linked `request_confirmation` also stayed `pending`,
so the board kept offering an authorization link for a flow the user had
already declined and the requesting agent never learned the answer.
Route every terminal callback — success, denial, cancel — through one
`consumeOAuthState` helper. Two properties pull in opposite directions
and both matter:
- a stranger's callback must not *consume* the request, so the row is
loaded and bound to the caller (expiry, actor type/id, session, or the
subject user) before anything is deleted, and a failed binding check
leaves the victim's flow live and completable;
- a replayed callback must not *complete* the request, so the delete is
the single statement that decides ownership — `DELETE … RETURNING`
hands the row to exactly one of two concurrent callbacks and the loser
is told the state is spent rather than exchanging a code against it.
A denial then resolves any linked interaction as `rejected`, scoped to a
still-`pending` row so a board user who answered the card directly keeps
their own answer, with a Paperclip-authored reason rather than the
provider's.
The message and its `providerError` label are PAP-17108's, unchanged.
Five regression tests, both negative-controlled: a code obtained before
a denial can no longer be exchanged; an unrecognized cancel code behaves
the same; another session's or another user's denial is refused without
spending the request, which the owner then completes; the board's prompt
ends `rejected`; and two simultaneous callbacks produce exactly one
token exchange. That last one needs a second database handle — this
driver pipelines everything issued through one handle, which serializes
the two callbacks and makes a non-atomic delete pass — and it has to
connect before racing, or the first callback wins uncontested.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A generic remote MCP connection points at an arbitrary authorization server, so
`error`, `error_description`, `error_uri` and the response body are all
attacker-chosen. Paperclip was returning the provider's `error_description`
verbatim as the public error message on the callback-denial, token-exchange and
dynamic-client-registration paths, and the callback did it before validating the
state at all. A hostile provider could therefore plant a canary secret, ANSI
escapes, or "paste your recovery key here" instructions into API responses,
board UI copy, and 4xx log lines — in Paperclip's own voice.
The operator now only ever reads text Paperclip authored:
- `normalizeOAuthProviderError` reduces the provider's `error` to a bounded,
allowlisted label (RFC 6749 §4.1.2.1/§5.2, RFC 7591 §3.2.2, OIDC Core
§3.1.2.6). An unrecognized code collapses to `unrecognized` rather than being
echoed, since the label is still untrusted input.
- `oauthProviderErrorMessage` maps that label to stable Paperclip copy. The
label plus the upstream HTTP status stay in structured `details` for
diagnosis; the description and body are never read on any path.
- `error_description` is gone from the callback route and from the
`completeOAuthCallback` input type, so there is nothing left to reflect
through, and `redactSensitive` scrubs it from the request query that
`customProps` copies into 4xx log lines.
- The callback validates the state binding and the actor before acting on a
provider-reported failure, and consumes the state so a denial is terminal.
Refresh goes through the same token-exchange path, and revocation makes no
provider call, so both are covered. Tests plant a canary secret, ANSI escapes
and markdown in `error_description` and assert none of it reaches an API
response, a thrown message, a console line, an audit row or an activity detail;
both assertions were negative-controlled by restoring the reflection.
PAP-17108. Finding 3 of the PAP-17088 security review.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Pinning to `approved[0]` alone dropped the failover `fetch` gave a
multi-homed host, so one dead A record took the whole connection down.
Walk the approved addresses in resolution order instead — but only for
addresses that are unreachable. A peer that answers from an unapproved
address is the rebinding defence firing, not a reachability problem, so
it still fails closed on the first try rather than being retried past.
Also close the gaps the response deadline left behind:
- a connect timeout used to leak the half-open socket, since nothing
above that point owned it yet;
- both deadlines threw bare `Error`s, so callers that switch on the
guard's error codes saw a timeout as an unclassified failure — they
now carry `remote_http_response_timeout`;
- a finished response left an armed socket timer behind; and
- the default deadline drops from undici's 300 s to 30 s, which is
already generous for metadata discovery, a token exchange or a
JSON-RPC round trip. `tools/call` owns a caller-set budget an operator
can raise to 60 s, so it passes `responseTimeoutMs` explicitly rather
than being truncated by the tighter default.
Covers each with a regression test, including a peer that dribbles
header bytes forever — the case that rules out `req.setTimeout`, whose
idle clock such a peer can reset indefinitely.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A generic remote MCP connection lets the server it points at advertise its own
authorization-server metadata, and `authorization_endpoint` was accepted as an
arbitrary string: `startOAuth` built a `URL` from it and the board passed the
result to `window.location.assign`. A hostile server could therefore advertise
`javascript:`/`data:` to run script in the board's origin, `file:` to read a
local path, or plain `http:`/a credential-bearing URL to downgrade or disguise
where the operator was being sent.
Parse every OAuth endpoint through one shared validator instead — discovered
metadata, `WWW-Authenticate` hints, pasted/persisted config, and gallery
defaults alike. It requires https (loopback http only under the existing
local-development policy, plus this deployment's own origin, which is how the
smoke-lab fixture is served), and rejects embedded credentials, fragments and
malformed values. Discovery drops an unusable endpoint so a second advertised
authorization server still gets a chance, and raises the refusal only if nothing
safe was found — so an unsafe value is never persisted and never reported as a
bare "does not advertise OAuth sign in". The token and registration endpoints go
through the same gate, so a row written before this existed cannot send a code or
client secret over plaintext.
The board applies the same validator at the navigation boundary rather than
trusting the response body, and — since a valid HTTPS page can still phish —
names the authorization host on the redirect screen while keeping the
"Unverified server" label.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The pinned transport replaced platform fetch, and with it undici's
headersTimeout/bodyTimeout. Only the gateway tools/call path passes an
AbortSignal; the OAuth discovery, registration and token-exchange callers
pass none, so a remote server that accepted the connection and then stayed
silent would hold the request open indefinitely. That is a hang the code
this replaced did not have.
Adds a 300s default matching undici, applied as a hard headers deadline
(not req.setTimeout, which is socket-idle based and a dribbling server
could reset forever) plus an idle deadline between body chunks so a
half-sent body rejects in response.text() instead of hanging the caller.
Both new tests hang to the vitest timeout when the deadlines are removed.
Refs PAP-17098
assertPublicRemoteHttpEndpoint resolved an operator-supplied hostname,
approved it, and then handed the *hostname* to global fetch, which
resolved it a second time. A name server the attacker controls can answer
with a public address for the guard's lookup and a loopback, RFC 1918 or
link-local address for the connection, so an authenticated public
deployment could be walked onto internal services or cloud metadata.
The guard now returns the address set it approved, and
guardedRemoteHttpFetch dials that address itself: one resolution, no
second lookup below it, the original hostname kept for Host, TLS SNI and
certificate identity, and a peer-address check before a single request
byte (or ClientHello) is written. Redirects stay manual so the caller
re-runs the whole guard on every hop it follows.
Wired into all three guard-then-fetch sites: OAuth discovery/registration/
token exchange, catalog tools/list, and the gateway tools/call path.
Two cases keep platform fetch, both because there is nothing to pin:
deployments that allow private endpoints, and URLs that already carry an
IP literal.
Regression coverage models the rebinding name server deterministically —
public on the first answer, 127.0.0.1 after it — and proves the request
lands on the approved address while the loopback listener sees no
connection at all. Reintroducing either half of the fix (a second lookup
at dial time, or dropping the peer check) turns those tests red.
Residual risk unchanged: egress allowlisting and network policy are still
recommended as defence in depth.
Refs PAP-17098
Add GENERIC-REMOTE-MCP.md: the two operator routes, what the guided flow does
with each kind of endpoint, header-safety rules, and the registration preference
order with its client-binding and re-registration behaviour.
Correct the connector playbook, which read as though a curated `AppDefinition`
were required and as though `ownershipModes` gated registration generally. It now
opens by saying a catalog entry is the optional branded convenience layer, and its
DCR section describes all four registration tiers, the public-HTTPS requirement
for CIMD, the issuer/resource/callback/company binding, and the path-aware
discovery and `iss`/`resource` conformance the broker now implements.
Note on the PostHog doc that the curated flow is preferable but not required —
which is exactly what the acceptance demo has to establish.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
