Deactivate surviving grants and shared connections whose credentials belonged to a suspended or archived member so dependent runs fail closed with an actionable reauthorization state.
Co-Authored-By: Paperclip <noreply@paperclip.ing>
Sweep delegated personal access when a membership is suspended and prune every surviving reference before destroying the user's secrets.
Co-Authored-By: Paperclip <noreply@paperclip.ing>
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - The instance settings surface (Access, Plugins, Adapters, General,
Experimental) assumes the person at the keyboard operates the whole
instance
> - Operators who host Paperclip for others — a managed cloud or an
internal shared server — expose settings pages and toggles that do not
apply to their deployment, and the related mutation APIs stay open
> - A hosted tenant can open Plugins or Adapters, try an action, and hit
a confusing failure, because only a few hardcoded platform floors exist
> - This pull request adds a generic, operator-configured visibility
mechanism: one env var hides declared settings surfaces in the UI and
floors their mutation routes with a stable 403 code
> - The benefit is a clean hosted-tenant settings surface for any
operator, with zero behavior change for normal self-hosted instances
## Linked Issues or Issue Description
**Subsystem affected**
Instance settings (server routes and UI), the shared settings registry
in `packages/shared`, and the `/api/health` bootstrap payload.
**Problem or motivation**
An operator who hosts Paperclip for other people cannot hide settings
surfaces that the platform manages. Tenants see Access, Plugins, and
Adapters pages, backup retention, and host-level experimental toggles
that do nothing useful for them. The mutation APIs behind these surfaces
also stay open, so a tenant admin can attempt actions the platform must
control. ROADMAP.md names a cleaner shared deployment story as a goal
("Teams should be able to run the same product in hosted or semi-hosted
environments without changing the mental model").
**Proposed solution**
Add a declarative registry of hideable settings surfaces and one env
var, `PAPERCLIP_HIDDEN_SETTINGS`. The server parses the list at boot,
reports it on `/api/health`, and rejects value-changing writes to hidden
surfaces with a stable `settings_operator_managed` 403 code. The UI
reads the list from the health payload and removes the hidden pages,
sections, and toggles from navigation, routes, and page content. Unknown
keys warn and are ignored, so one list can roll across a fleet with
mixed app versions. With the variable unset, behavior is byte-identical
to today.
**Alternatives considered**
- Hardcode the hidden set for cloud instances in this repo: rejected,
because each hosting operator needs a different policy, and policy does
not belong in shared code.
- Deliver the hidden set through the managed-config document: rejected,
because that channel is cloud-specific and fail-closed on unknown
fields; a plain env var works for any operator, including self-hosted
shared servers.
- Lock the controls with a badge instead of hiding them: rejected for
these surfaces, because they are meaningless to tenants, not merely
platform-controlled; the existing managed-overlay lock stays the right
tool for controlled flags.
**Roadmap alignment**
Supports the "shared deployment story" item in ROADMAP.md: hosted and
semi-hosted deployments keep the same product with a settings surface
that matches what the tenant can actually do.
## What Changed
- New `packages/shared/src/settings-visibility.ts`: registry of hideable
surfaces (every instance settings page — profile, environments, access,
heartbeats, experimental, plugins, adapters; every Instance → General
section; every experimental flag as `instance.experimental.<key>`), the
`PAPERCLIP_HIDDEN_SETTINGS` parser, and the `settings_operator_managed`
error code. The General page stays visible as the settings root and
redirect target.
- New `server/src/services/settings-visibility.ts`: parse-once accessor;
unknown keys log one warning and are ignored.
- `/api/health` reports `hiddenSettings` on every response shape; the
field is omitted when nothing is hidden.
- Server floors on hidden surfaces, with same-value echo tolerance (the
`executionMode` precedent): field-backed general sections and
experimental keys reject value-changing PATCHes, and hiding the whole
Experimental page floors every toggle; plugin lifecycle and config
writes, adapter management writes, and the Access admin routes (reads
included) return 403 `settings_operator_managed`. Reads the app itself
needs (plugin `ui-contributions`, adapter metadata, plugin job trigger)
stay open. Pages without instance-scoped mutation routes are hidden in
the UI only.
- UI: new `useHiddenSettings` hook and `HiddenSettingsPageGate` route
gate (hidden pages redirect to the settings root); the settings sidebar
and tab bar drop hidden entries; remembered settings paths remap to the
default page; `InstanceGeneralSettings` skips hidden sections; every
`ExperimentalToggleCard` now carries its flag key and renders nothing
when hidden.
- Removed the dead `InstanceSidebar` component (referenced only by its
own test).
- Docs: `docs/deploy/environment-variables.md` documents the variable
and the key registry.
## Verification
- `pnpm vitest run` over the new and extended suites: shared registry
and parser, representative floor tests per route class (changed-value
403, same-value echo 200, unset env 200, page-level Experimental
hiding), the health field, the route gate, nav filtering, and
section/card hiding with one hidden example per surface kind — 168 tests
pass.
- Full root `pnpm typecheck` passes.
- Manual: booted a server with the variable set. `/api/health` lists the
keys; an unknown key logs one warning and the server boots; hidden pages
redirect; hidden sections and cards do not render; hidden-field PATCH
returns 403 with `details.code = "settings_operator_managed"`; a
same-value echo returns 200. Unset the variable: the full settings
surface returns and responses are byte-identical to master.
## Risks
- Low risk for self-hosted instances: with the variable unset, the
hidden set is empty, the health field is omitted, and no floor
activates.
- Flooring plugin config writes assumes hosted deployments configure
plugins through the platform. If a future bundled plugin needs
tenant-entered config, the floor needs a narrow carve-out.
- Hidden-key floors tolerate same-value echoes, so API clients that
round-trip full GET responses keep working.
- Hiding a toggle does not change its value; operators pair hiding with
the desired default where the value matters.
## Model Used
Claude Fable 5 (Anthropic, `claude-fable-5`) with extended thinking and
agentic tool use, driven through the Claude Code CLI (file edits, test
execution, and live-server verification loops).
## Checklist
- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change (e.g. `docs/...`, `fix/...`)
and contains no internal Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [ ] All Paperclip CI gates are green
- [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [x] I will address all Greptile and reviewer comments before
requesting merge
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work.
> - The heartbeat system records agent runs and can add a run summary to
an issue.
> - The ACPX engine receives output text and internal thought text as
separate streams.
> - The default summary strategy joined both streams and could publish
internal text in an issue comment.
> - Paperclip already has final-output segmentation for run summaries.
> - This pull request makes final-output-only summaries mandatory and
removes the configuration bypass.
> - The benefit is that automatic issue comments contain the intended
final message instead of internal execution text.
## Linked Issues or Issue Description
Refs #11761
**What happened?**
The ACPX engine used the full summary strategy when an adapter did not
set `summaryStrategy`. That strategy joined all text deltas, including
thought-stream text and intermediate narration. The heartbeat finalizer
could then store that summary as an issue comment.
**Expected behavior**
An automatic issue comment must use only the final output segment.
Configuration must not allow thought-stream text or intermediate
narration into that summary.
**Steps to reproduce**
1. Run an ACPX adapter without a configured `summaryStrategy`.
2. Emit an output delta, a thought delta, a tool call, and a final
output delta.
3. Read the generated run summary.
4. Observe that the old default included all text deltas.
**Paperclip version or commit**
`54b8bec44417511c623999613f9f1006f8af0517`
**Deployment mode**
Built from source with a local ACPX adapter.
## What Changed
- Limit ACPX run summaries to the final non-empty output segment.
- Ignore the legacy full-summary setting so configuration cannot bypass
containment.
- Update regression tests for the safe default and an attempted unsafe
override.
## Verification
- Observed the new guard fail before the implementation change because
the summary contained thought text.
- Ran `pnpm exec vitest run
packages/adapter-utils/src/acpx-engine/execute.test.ts -t "defaults run
summaries to the final output segment without thought text|does not
allow configuration to include thought text in run summaries"`. Result:
2 passed.
- Ran `pnpm exec vitest run
packages/adapter-utils/src/acpx-engine/execute.test.ts`. Result: 130
passed.
- Ran `pnpm --filter @paperclipai/adapter-utils typecheck`. Result:
passed.
## Risks
- Run summaries are shorter for adapters that relied on full text
aggregation.
- The old `summaryStrategy: "full"` setting no longer changes summary
behavior. This is an intentional containment change.
- The change does not alter run logs or tool events. It changes only the
summary selected for downstream use.
> This is a focused security and privacy bug fix. It does not add
roadmap scope.
## Model Used
- OpenAI Codex on the GPT-5 family. The runtime did not expose the exact
model ID or context-window size. Reasoning, tool use, terminal
execution, and code editing were enabled.
## Checklist
- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change and contains no internal task
id
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] I have updated relevant inline documentation to reflect my changes
- [x] I have considered and documented any risks above
- [ ] All Paperclip CI gates are green
- [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [ ] I will address all Greptile and reviewer comments before
requesting merge
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work.
> - The control plane must keep each active issue on a clear execution
or recovery path.
> - A missing issue disposition can require more than one bounded repair
attempt.
> - A server restart could lose that repair path or move source
ownership to the recovery owner.
> - A parked or expired retry could also make the user interface show a
false healthy state.
> - Concurrent recovery loops must not schedule the same repair attempt
twice.
> - This pull request keeps retry state durable, makes scheduling
atomic, and keeps source ownership stable.
> - The benefit is that recovery continues after a restart and operators
see the correct state.
## Linked Issues or Issue Description
**What happened?**
A run that ended without a valid issue disposition could lose its repair
path after a server restart. Manager recovery could also change the
source owner. In addition, a parked or expired retry could make the
issue look healthy when no active work existed. Concurrent
reconciliation could also schedule the same repair attempt twice.
**Expected behavior**
Paperclip must keep bounded source and manager repair attempts across
restarts. Recovery ownership must stay separate from source issue
ownership. The server and user interface must report only a live retry
as active work. Each repair attempt must be scheduled at most once per
company.
**Steps to reproduce**
1. Start an agent run on an issue.
2. End the run without a valid issue disposition.
3. Let the first repair attempt schedule a retry.
4. Restart the server, let the retry time pass without a live run, or
start two reconciliation loops together.
5. Observe that the repair path can stop, the issue can show a false
healthy state, or duplicate retries can be created.
**Paperclip version or commit**
The problem existed on `master` before candidate head
`d8e620fe86bade7df18decac332007f5821ae04f`.
**Deployment mode**
The problem affects self-hosted servers and local builds that use
automatic recovery.
## What Changed
- Persist bounded source-owner and manager repair lineages with stable
fingerprints and retry limits.
- Resume incomplete disposition repairs after a server restart.
- Keep recovery ownership separate from source issue ownership and
enforce source mutation authority.
- Project live retry evidence into issue and blocker summaries.
- Show recovery owner, return owner, attempt count, and retry state in
the board user interface.
- Treat expired or parked retries as attention states unless a queued or
running attempt exists.
- Atomically deduplicate disposition-repair wake requests with a
company-scoped partial unique index.
- Reuse the winning run when concurrent reconciliation loses the
uniqueness race, without duplicate scheduling activity.
- Honor disabled on-demand wake policy before recovery scheduling and
again before delayed retry promotion.
- Keep the new index migration safe for lagging seeded databases that
already contain the index.
- Add server and user interface tests for recovery, restart, ownership,
retry, concurrency, and blocker states.
- Update the implementation and execution semantics documents.
## Verification
- Focused server recovery and ownership suites: 282 tests passed on the
repaired base candidate.
- Focused user interface recovery suites: 128 tests passed on the
repaired base candidate.
- Atomic-deduplication schema and recovery suites: 111 tests passed on
the first Greptile repair.
- Recovery and scheduled-retry wake-policy suites: 126 tests passed at
`d8e620fe86bade7df18decac332007f5821ae04f`.
- The exact lagging-source migration-order test passed after the index
migration became idempotent: 1 test passed and 62 unrelated tests were
skipped.
- `@paperclipai/db` and `@paperclipai/server` typechecks passed at the
current head.
- Migration generation and migration safety checks passed for migration
`0226_tan_colossus.sql`.
- `pnpm check:token-gates` passed on the repaired base candidate.
- `pnpm -r typecheck` passed on the repaired base candidate.
- `pnpm build` passed on the repaired base candidate.
- `pnpm test:run` passed 4,540 tests on the repaired base candidate.
Four fixed-port cases met listeners that already existed on the host.
- The two unchanged fixed-port files passed in an isolated network
namespace: 129 tests passed and 27 tests were skipped.
- Independent Security and QA reviews approved
`63c0423aab54c66f2293a20b0fb3f3b013ee3ba8`; exact-head re-review is
required after automated checks settle on
`d8e620fe86bade7df18decac332007f5821ae04f`.
## Risks
- Recovery orchestration affects issue liveness and ownership. The new
paths use bounded attempts, stable fingerprints, row locks, authority
checks, and database uniqueness.
- A conservative attention state can show more warnings when a scheduled
retry has no queued or running attempt. It does not hide stopped work.
- Migration `0226_tan_colossus.sql` creates a partial unique index on a
known-large table. Migrations run transactionally, so `CONCURRENTLY` is
unavailable. The matching disposition-repair key namespace is introduced
by this release, so deployed databases have no matching rows before the
index is added.
> For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and
discuss it in `#dev` before opening the PR. Feature PRs that overlap
with planned core work may need to be redirected — check the roadmap
first. See `CONTRIBUTING.md`.
## Model Used
- OpenAI Codex from the GPT-5 model family used agentic reasoning, tool
use, and code execution. The runtime did not expose the exact model ID
or context window.
- Anthropic Claude Opus 5 used a 1M context window, tool use, and code
execution for part of the user interface repair, as recorded in the
commit history.
## Checklist
- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change (e.g. `docs/...`, `fix/...`)
and contains no internal Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [x] All Paperclip CI gates are green
- [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [x] I will address all Greptile and reviewer comments before
requesting merge
---------
Co-authored-by: Paperclip <noreply@paperclip.ing>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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>
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
Dotta