[patrickbrandao]

Closes #74

Item 2 (setup status probing Ollama) was done by #113 and is covered by no-phonehome-egress.e2e-spec.ts; skipped here.

What changed

1. Boot backfills on the direct URL. BootBackfillConnection (database/backfills/boot-backfill-connection.ts): when MIGRATION_DATABASE_URL, DATABASE_UNPOOLED_URL or BACKFILL_DATABASE_URL is set, MessageProviderBackfillBootService and RequestBackfillBootService take their session advisory lock and run their SQL on a lazily opened direct DataSource (max 2 connections) instead of the DATABASE_URL pool (PgBouncer in transaction mode). It closes when the backfill (and the tail sweep) finishes and at shutdown. Without a direct URL nothing changes. The URL is never logged.
2. CIMD opt-in DNS failure is pinned. With THINKER_CIMD_ALLOW_PRIVATE=true, a failed up-front lookup no longer lets the request resolve on its own: it resolves again at connect time through a validating lookup that refuses cloud-metadata answers and pins the validated address, as on the default path.
3. Fetch error causes are kept. ProviderClient.executeFetch keeps a sanitized cause (message and code, key= redacted) on the error it rethrows, so a connect failure is no longer just fetch failed in the logs.
4. SSRF-blocked provider URL → 502 provider_url_blocked. The forward-time revalidation refusal and a connect-time SsrfBlockedError (the cause of undici's fetch failed) become ProviderUrlBlockedError. It is handled as a transport failure, so fallbacks run, and the response is HTTP 502 with error.code/type provider_url_blocked and a message saying the provider URL was blocked by the SSRF policy, instead of the 500 internal error.
5. ssrfSafeFetch cast is commented (undici's Response is not the global class; callers only use the interface).
6. Outbound proxy documented in the guide: ssrfSafeFetch ignores HTTP(S)_PROXY / NODE_USE_ENV_PROXY on purpose, since a forward proxy would bypass the pinned address.

Tests

• Unit: new boot-backfill-connection.spec.ts (precedence, lazy single open, close, no URL in logs, both services lock and run on the direct DataSource), CIMD connect-time validation and pinning, provider-client SSRF mapping and cause sanitization. npx jest src/database/backfills src/auth/cimd-client src/routing/proxy src/common/utils/ssrf: all passed; full backend run in the report.
• npm run test:e2e:docker -- test/custom-provider-dns-rebinding.e2e-spec.ts test/mcp-cimd-ssrf.e2e-spec.ts test/custom-providers.url-validation.e2e-spec.ts test/request-backfill-fallbacks.e2e-spec.ts test/user-provider-backfill-migrations.e2e-spec.ts: 5 suites, 51 passed.
• tsc -p tsconfig.test.json, eslint: clean.

qa-docker: yes. Suggested: (a) a custom provider whose host resolves to a metadata address returns 502 provider_url_blocked (streaming and non-streaming); (b) boot with MIGRATION_DATABASE_URL pointing at the direct Postgres and DATABASE_URL through a pooler, check the backfill log line "post-deploy backfills use the direct database URL"; (c) the pinned forward with streaming through a regional override, from the issue.

🤖 Generated with Claude Code

[patrickbrandao]

Full backend unit run (npx jest --maxWorkers=4): 476 suites, 9095 tests passed.

[patrickbrandao]

Review: #156 fix(providers): direct-URL boot backfills, pinned CIMD fallback lookup, 502 for blocked provider URLs

Reviewed git diff origin/main...3bcba8b.

High risk: this PR changes the SSRF defenses of the OAuth/MCP authorization flow (packages/backend/src/auth/cimd-client-metadata-fetch.ts) and adds a new database connection built from credential-bearing URLs (MIGRATION_DATABASE_URL / DATABASE_UNPOOLED_URL / BACKFILL_DATABASE_URL). Criterion: "changes auth, crypto or key handling". The maintainer decides this merge.

Blockers: none. The new proxy error handling (ProviderUrlBlockedError, the 502 mapping, the sanitized cause) is covered by unit tests in provider-client.ssrf.spec.ts.

Should-fix

1. No e2e for the 502 through the proxy. packages/backend/src/routing/proxy/proxy-transport.ts:71,127 / provider-client.ts:976. isTransportError now treats a blocked URL as a transport error, so proxy-fallback.service.ts (lines 478, 527, 753) runs fallbacks and returns the 502, in both streaming and non-streaming. That client-visible proxy behavior has only unit tests, and test/custom-provider-dns-rebinding.e2e-spec.ts does not assert status 502 or provider_url_blocked. Failure scenario: a later refactor of the fallback service maps the error back to 500, or stops falling back, and only the unit tests notice. Fix: add an e2e case to the DNS-rebinding suite (custom provider rebinding to 169.254.169.254, non-streaming and streaming, with and without a fallback) that asserts the 502, the code and that the fallback runs.

Nits

• packages/backend/src/database/backfills/boot-backfill-connection.ts:65-75: if close() runs while open() is still pending (shutdown right after boot), open() sets this.direct afterwards and the 2-connection pool leaks until the process exits. Fix: in close(), await this.opening before destroying, or have open() check a closed flag.
• boot-backfill-connection.ts:25: each of the two boot services opens its own pool (max: 2), so up to 4 extra direct connections. Acceptable, but worth a line in the guide.
• cimd-client-metadata-fetch.ts createValidatingLookup: correct. It validates and pins the connect-time answer with the same selectPinnedAddress rule, so the old path that skipped validation is closed. Good fix.

Security checks

• The direct URL is never logged (tested). Error messages are redacted (key=***), and the forwarded cause keeps only its message and code.
• The 502 message includes the validator's detail (host / "cloud metadata address"). It goes only to the tenant that configured the URL, so there is no cross-tenant leak.
• No new outbound destination.
• CI on 3bcba8b: all green.
• qa-docker: required. The three suggested checks (502 streaming and non-streaming, direct-URL backfill boot, pinned regional streaming) have not been reported yet. They are a merge gate.

Under aprovado each should-fix would need to be fixed or tracked. Here, should-fix 1 must be fixed or ticketed before the maintainer merges.

Head revisado: 3bcba8b
Veredito: risco alto (aguardar mantenedor)

[patrickbrandao]

Review follow-ups are tracked in #158.

[patrickbrandao]

qa-docker: PR #156 (FAIL on one item)

Image thinker:qa-156 built from the PR head (linux/arm64, 428.8 MB, build 1m). PostgreSQL 16.15 (postgres:16-alpine). Isolated Compose project, run-scoped volumes, mock provider only, THINKER_CATALOG_SYNC=false.

Step	Result
Boot, health, SPA, setup, agent key	pass
/v1/models, OpenAI chat, Anthropic /v1/messages, Responses via mock	pass
contracts/gateway (9 tests)	pass (9/9)
Boot with MIGRATION_DATABASE_URL (direct URL, tagged application_name=qa-direct): log "post-deploy backfills use the direct database URL", both agent_message_provider_attribution and requests_agent_messages_v1 completed, direct connection closed afterwards	pass
Custom provider whose host resolves to 169.254.169.254 (extra_hosts entry in the container; created with a good URL, then base_url changed by SQL because the create API already refuses it) returns HTTP 502 for non-stream chat, stream chat and /v1/messages	pass
Same requests expose error.code = provider_url_blocked to the client	FAIL
Backend e2e (test:e2e:docker)	pass (71 suites, 599 tests, 449s)
Teardown, no run-labelled volumes left	pass

Failure: the client never sees provider_url_blocked

The upstream-side body is correct (gateway log):
body={"error":{"message":"Provider URL blocked by the SSRF policy: ...","type":"provider_url_blocked","code":"provider_url_blocked"}}

but every client response (stream and non-stream, OpenAI and Anthropic format) is:

HTTP 502 {"error":{"message":"Upstream provider returned bad gateway","type":"server_error","param":null,"code":null,"status":502,"source":"provider","provider":"custom:<id>","model":"custom:<id>/mock-model"}}

Diagnosis: the proxy response handler passes the 502 body through sanitizeProviderError (packages/backend/src/routing/proxy/proxy-error-sanitizer.ts), which collapses 5xx bodies to the generic text in production and only keeps structured data for the context-length classification, so code/type are dropped. The unit test in provider-client.ssrf.spec.ts asserts the code on the provider-client response, not on the final client-facing body, so it does not catch this. The sanitizer needs to recognise provider_url_blocked (like classifyProviderError does for context_length_exceeded) and keep its code, type and a safe message.

Tested SHA: 3bcba8b

[patrickbrandao]

Re-review: #156 (delta since 3bcba8b)

Reviewed git diff origin/main...94ec3c8 and the delta from 3bcba8b: a merge of main, the provider_url_blocked handling in proxy-error-sanitizer.ts, the proxy e2e and the close()/open() race fix.

Previous findings

• Should-fix (e2e for the 502): fixed. test/custom-provider-dns-rebinding.e2e-spec.ts now asserts a 502 with provider_url_blocked for non-streaming, streaming and the Anthropic Messages surface. It also checks that the fallback chain still runs (200 from the fallback), that 169.254 is never echoed and that the metadata trap gets zero connections.
• Nit (close() racing open()): fixed. boot-backfill-connection.ts close() now awaits an in-flight opening before destroying the pool. get() clears opening only if it is still the same promise.

New code

• packages/backend/src/routing/proxy/proxy-error-sanitizer.ts:221: a 502 whose body carries provider_url_blocked keeps the code through the production 5xx collapse, with a fixed message, so the hostname and IP are not exposed. This fixes the qa-docker failure, and unit tests plus the e2e above cover it.

Nits

• proxy-error-sanitizer.ts:221: the classification keys on the body, so a real upstream (for example a hostile custom endpoint) that answers 502 with {"error":{"code":"provider_url_blocked"}} would be shown as blocked by the SSRF policy. The impact is cosmetic and confined to that tenant's own provider. A stricter version would match only the gateway's synthetic response (for example a marker set by buildTransportErrorResponse).
• boot-backfill-connection.ts: a get() called after close() during shutdown could reopen a pool. The services set stopping first, so the window is very small.

Blockers: none. Should-fix: none open.

High risk (unchanged): this PR changes the SSRF checks in the OAuth/MCP CIMD metadata fetch and adds new DB connections from credential-bearing URLs (criterion: auth/key handling). The maintainer decides this merge.

CI on 94ec3c8: all 20 checks green. qa-docker must pass again on this head, because the sanitizer change came out of a qa-docker failure.

Head revisado: 94ec3c8
Veredito: risco alto (aguardar mantenedor)

[patrickbrandao]

qa-docker re-check (image built from this head)

Image thinker:qa-b035d3 (arm64, 427 MB, build 1m15s). Bases: node/distroless per docker/Dockerfile; Postgres 16.15 (postgres:16-alpine, pinned digest in compose).
Scenario: provider created with a good URL, then pointed by SQL at blocked.qa.test, mapped to 169.254.169.254 with extra_hosts. Mock provider on the host; no external hosts.

Check	Result
1. Non-stream chat: 502, error.code = provider_url_blocked, fixed message	PASS
1. Stream chat: same	PASS
1. /v1/messages (stream and non-stream): same, Anthropic error envelope	PASS
1. Resolved address (169.254.x) absent from every response	PASS
2. Fallback to the mock configured: chat, stream, /v1/messages, /v1/responses (stream and non-stream) succeed through the fallback	PASS
3. Smoke: health, dashboard HTML, first-admin setup, login, agent key, /v1/models	PASS
4. Boot with MIGRATION_DATABASE_URL: log shows "post-deploy backfills use the direct database URL"; request backfill ran and completed. The provider-attribution backfill logged "another backfill is running; retrying later" (same as the default boot)	PASS
5. contracts/gateway (9 tests, with agent key)	PASS

Response body for the blocked cases (chat):
{"error":{"message":"Provider URL blocked by the SSRF policy. Check the base URL of this provider.","type":"provider_url_blocked","param":null,"code":"provider_url_blocked","status":502,"source":"provider",...}}

Notes: the item 4 backfill was re-triggered by deleting the backfill_state rows before the recreate. Volumes used the run-unique names thinker-qa-b035d3_* and were removed with a labeled down -v. Ports 5432/5433 untouched. The e2e suite and unit tests were not part of this re-check.

Tested SHA: 94ec3c8