fix(security): prevent outbound webhook SSRF

[seonghobae]

Security and buyer outcome

Refs #551. Protected develop still ships the historical outbound-webhook behavior until this PR integrates. This branch replaces that boundary with a bounded public-HTTPS transport, closes DNS-rebinding and Request-object bypasses, preserves signing/retry accounting, hardens the adjacent OIDC callback path, bounds public authentication request bodies, and keeps existing insecure webhook rows fail-closed with a tenant-visible replacement action.

The latest repair also removes a startup availability hazard in the legacy-webhook reconciliation path: a database whose active webhook destinations already comply with current policy no longer reserves SQLite's single writer merely to discover that no migration work is required.

Exact current state

• protected live base: develop@2c328875e00e86537df3e965170be80532571cad;
• exact contributor head: 92487d1f9e5215cd4b7302275c23393596799ba3;
• current synthetic merge ref: 948d8484c27e456e6ad7709dca4df92ca6c0b272;
• branch: fix/webhook-ssrf-551;
• Draft and mechanically mergeable at the latest fresh refetch;
• all currently posted review threads are resolved after executable repair or fresh verification;
• no qualifying independent current-head approval is present; and
• repository/organization checks are being re-evaluated on this head. The current Server Tests run is terminal success, while central SAST/Security evidence remains non-authoritative until the known synthetic-checkout control defect is repaired.

Any contributor-head or protected-base movement invalidates head/base-sensitive evidence until the exact revisions, review state, actual checkout attestations, checks, and live-base identity are freshly revalidated.

Implemented security and reliability contract

• webhook registration requires canonical public HTTPS and rejects credentials, fragments, localhost-like names, denied literals, private/special-use DNS answers, and mixed public/private answer sets;
• each delivery attempt resolves and validates public A/AAAA candidates, pins the HTTPS socket to an authorized address, preserves hostname/SNI, disables pooling, and refuses redirects;
• pre-handshake connection failure may fall back only to another already-authorized address; once TLS or response progress makes replay unsafe, the same mutating signed request is not resent to another candidate;
• signed webhook egress is routed through a module-local, fail-closed core outbound boundary rather than a process-wide globalThis.fetch replacement; unclassified core egress fails closed;
• webhook-registration and public signup/login payloads use direct bounded stream reads instead of teeing attacker-controlled streams with Request.clone();
• signup/login preserves the sole matching historical mixed-case stored identity while preventing new case-only duplicates;
• audit pagination is normalized to a positive bounded integer at the public facade;
• OIDC discovery, token, and JWKS requests use validated public-destination transport, redirect rejection, bounded response handling, cancellation, and provider deadlines;
• ID tokens require RS256 signature verification plus issuer, audience/azp, expiration, not-before, issued-at, subject, nonce, non-empty verified email, and stable subject/account-linking checks before session creation;
• OIDC state storage is bounded and expires fail closed; signing evidence is short-lived and signing keys are cached per (jwks_uri, kid) with a fixed bound;
• session revocation remains centralized across bearer, calendar, SSE, and attachment transports;
• persisted legacy webhook rows rejected by the current deterministic registration policy are disabled fail closed with one idempotent tenant audit next action rather than silently continuing insecure delivery;
• legacy webhook startup reconciliation first performs a read-only policy scan and returns without opening a writer transaction when no row needs mutation; when mutation is required it re-reads under one BEGIN IMMEDIATE transaction before disabling rows and writing audit evidence; and
• development-only loopback compatibility remains explicitly gated instead of weakening production transport.

Latest test-first startup-lock repair

A realistic SQLite contention regression was added before the production change. It opens a compliant legacy-webhook database, has a second connection hold BEGIN IMMEDIATE, and requires migrateLegacyWebhookDestinations() to remain a read-only no-op rather than competing for the writer.

RED predecessor:

• contributor head 6228733d4676a69f596742be765ac95f2dd9ab76;
• Server Tests run 33125882051, unit-and-api job 98703805992;
• hosted log failed the new assertion with database is locked at migrateLegacyWebhookDestinations, proving the unconditional transaction was reachable.

GREEN current head:

• contributor head 92487d1f9e5215cd4b7302275c23393596799ba3;
• Server Tests run 33126053154 is terminal success;
• unit-and-api job 98704354241 is terminal success and explicitly reports both legacy HTTP webhook migration regression passed and legacy private HTTPS webhook migration regression passed together with the full unit/API suite;
• that job checked out synthetic merge 948d8484c27e456e6ad7709dca4df92ca6c0b272, logging Merge 92487d1f9e5215cd4b7302275c23393596799ba3 into 2c328875e00e86537df3e965170be80532571cad.

The repair is intentionally two-part. server/webhook_legacy_migration.mjs now uses a read-only preflight and reacquires/re-reads under BEGIN IMMEDIATE only if a policy-incompatible active destination exists. server/db.mjs removes the two duplicated unconditional startup transactions and delegates all deterministic legacy-webhook reconciliation to that helper. This changes the actual application startup path rather than merely making the helper test green.

Retained test-first repair evidence

The current branch also retains realistic RED→GREEN regressions for Request-object webhook bypasses, OIDC signature/claim validation and email_verified, OIDC metadata SSRF, multi-key JWKS reuse, public-auth body bounding, historical mixed-case identity compatibility, webhook transport framing/replay semantics, legacy HTTP/private-HTTPS webhook migration, facade/core request observability, and module-local fail-closed egress ownership. Review threads are resolved only after the relevant behavior is repaired or freshly shown not to be a current defect.

Current workflow evidence

For exact contributor head 92487d1f9e5215cd4b7302275c23393596799ba3, the latest fresh workflow inventory is:

• Server Tests 33126053154 — terminal success;
• Security Scan 33126053151 — terminal success;
• Dependency Review 33126053168 — terminal success;
• Fuzz 33126053133 — terminal success;
• OSV Scanner 33126053576 — terminal success; and
• SAST Semgrep 33126053159 — still in progress at the last fresh refetch and therefore non-passing until terminal substantive evidence exists.

These hosted results are behavior/regression evidence, not yet merge authority, because the Server Tests run above materialized a synthetic PR merge rather than the immutable contributor SHA. ScopeWeave PR #523 owns the repository-native Server Tests/coverage exact-head checkout prerequisite. ContextualWisdomLab/.github#1222 is the canonical central owner issue for reusable SAST/Security exact-head evidence. Once those controls integrate, this unchanged contributor head (or its freshly revalidated successor) must regenerate every applicable test, browser, coverage/docstring, CodeQL/SAST/security, dependency/supply-chain, package/provenance and review gate with actual checkout identity proven against the exact contributor SHA.

Standards and traceability

docs/doctoring/outbound-webhook-ssrf.md records protected-shipped versus active-PR truth, SSRF/DNS-rebinding threats, pinned-address decisions, redirect/error semantics, executable acceptance, rollback boundaries, and applicable primary standards/official references.

Merge gate

Do not mark Ready, merge, or enable auto-merge until the unchanged exact contributor head remains reconciled to freshly resolved protected develop, #523's repository exact-head control and the central SAST/Security exact-head repair are protected-shipped, every applicable repository and organization CI/browser/owned-coverage/docstring/CodeQL/SAST/security/dependency/supply-chain/package/provenance gate is substantively terminal-passing under corrected evidence contracts, valid unresolved findings are zero, and a qualifying independent current-head approval genuinely exists under live governance. Pending, queued, skipped-required, cancelled, absent, neutral-required, failed, stale, predecessor, synthetic-only, status-only, author-only, model-only, rate-limited, or infrastructure-only evidence is non-passing.

---

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

app_core.mjs에 SaaS API를 분리하고 app.mjs를 보안 facade로 변경했습니다. 웹훅에 HTTPS 강제, 공개 주소 검증, DNS 재검증, 소켓 고정, 리다이렉트 거부와 안정적 오류 처리를 추가했습니다. OIDC 검증과 관련 회귀 테스트 및 설계 문서를 추가했습니다.

Changes

웹훅 SSRF 방어 및 SaaS 앱 통합

Layer / File(s)	Summary
SaaS core API 구현 server/app_core.mjs	인증, 조직, 프로젝트, 협업, 결제, 웹훅, 검색, 첨부파일, 공유 및 수명주기 API를 추가했습니다.
웹훅 URL 및 HTTPS 전송 정책 server/webhook_transport.mjs, tests/unit/webhook-transport.test.mjs, tests/unit/public-https-transport.test.mjs	비HTTPS URL, 인증 정보, fragment, 로컬·사설 주소를 거부합니다. 매 전송 시 DNS 결과를 검증하고 검증된 주소로 HTTPS 연결을 고정합니다. 연결 실패 시 검증된 다음 주소를 시도하며 리다이렉트는 따르지 않습니다.
보안 앱 facade와 라우트 연결 server/app.mjs	core 앱에 요청을 위임합니다. 웹훅 등록 URL을 검증하고 서명된 웹훅 POST에 보안 transport를 적용합니다. OIDC 요청에는 timeout, 리다이렉트 거부, JWT 및 nonce 검증을 적용합니다.
회귀 테스트와 보안 기록 tests/api/*, tests/unit/*, package.json, docs/doctoring/outbound-webhook-ssrf.md, CHANGELOG.md	웹훅 등록·전송 정책, DNS rebinding, 오류 처리, OIDC 경계와 상태 용량, 입력 정규화, 정적 파일 allowlist 및 테스트 구성을 검증하고 보안 설계를 기록합니다.

Estimated code review effort: 5 (Critical) | ~120 minutes

Merge Risk: 🟠 High · up to 5bfb2

This PR changes outbound webhook delivery and OIDC login handling, but the current head can resend POST requests after a response has started, potentially duplicating webhook actions or breaking one-time login exchanges; OIDC discovery destinations also remain insufficiently bounded, and an OIDC configuration response contract disagrees with its tests. Required current-head checks and independent approval are not yet available, so merge should be blocked until these issues are fixed and evidence is refreshed.

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant AppFacade
  participant CoreApp
  participant WebhookTransport
  participant DNSResolver
  participant HTTPSRequest
  Client->>AppFacade: 웹훅 등록 또는 이벤트 요청
  AppFacade->>CoreApp: 인증·tenant·권한 검사
  CoreApp->>WebhookTransport: 서명된 웹훅 전달
  WebhookTransport->>DNSResolver: 매 시도 A/AAAA 조회
  DNSResolver-->>WebhookTransport: 주소 목록 반환
  WebhookTransport->>WebhookTransport: 공용 주소 재검증 및 소켓 고정
  WebhookTransport->>HTTPSRequest: redirect 없는 HTTPS POST
  HTTPSRequest-->>WebhookTransport: 응답 상태
  WebhookTransport-->>CoreApp: 전송 결과 또는 안정적 오류

Loading

Possibly related PRs

• ContextualWisdomLab/scopeweave#523: server/app.mjs와 웹훅/API 테스트가 겹치지만, 주요 목적은 SSRF 방어가 아닌 exact-head CI 검증입니다.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	제목은 아웃바운드 웹훅 SSRF 방지라는 PR의 주요 변경 사항을 정확하고 간결하게 설명합니다.

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/webhook-ssrf-551

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[seonghobae]

@opencode-agent Repair this existing Draft branch only. Exact current head is 006cfabda2f9e1b36221215a481b9475a07164c4; exact protected base remains develop@1fadec04195805722829b386475a09a15f8cd926. Do not create another #551 branch/PR and do not rebase/force-push. The current head is deliberately test-only RED: keep tests/api/webhook-destination-policy.test.mjs and its canonical API registration, wait for/record its hosted failure, then implement the #551 root-cause contract on this branch. Add modular URL/IP destination policy plus pinned HTTPS transport with deterministic tests for A+AAAA private/mixed answers, IPv4-mapped IPv6, DNS rebinding between attempts, redirect refusal, public success, timeout/retry cleanup, and zero network attempt for denied destinations. The actual connection must be bound to the address just validated while retaining original hostname/SNI; validation followed by an unrelated resolver decision is insufficient. Each retry freshly resolves/validates/pins. Preserve HMAC event signing/filtering, 3-second budget, delivery accounting, auth/tenant behavior, and public HTTPS path/query support. Add canonical c8/coverage-contract registration, beginner-readable JSDoc, doctoring/traceability with the primary references in #551, and update the PR body with exact RED→GREEN/current-head evidence. Before every push refetch this head/base and inspect any intervening delta; if another actor moved the branch, adapt rather than overwrite. Do not touch central .github, auth/session, Clearfolio, Stripe inbound webhook, lockfiles, branch protection, or unrelated files. Do not merge.

[seonghobae]

@coderabbitai review

[coderabbitai]

⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

@seonghobae I will review the specified current head against the specified develop base. I will focus on current-source security and account-integrity defects. I will not transfer predecessor findings or status.

⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[devin-ai-integration]

Devin Review found 3 new potential issues.

[devin-ai-integration]

Devin Review found 2 new potential issues.

[seonghobae]

Fresh current-head revalidation for the transient SQLite writer-lock finding:

• exact contributor head remains df1b14b83c24425a8e00d3825d898a5b122a08ad on fix/webhook-ssrf-551;
• protected live base remains develop@2c328875e00e86537df3e965170be80532571cad;
• the existing realistic regression in tests/api/webhook-legacy-migration.test.mjs is unchanged across the predecessor/fix boundary and uses a separate child process to hold a WAL BEGIN IMMEDIATE writer lock for 300 ms before release.

RED is preserved rather than inferred: predecessor 3fc64ee2d8407739c720511101bff9066df15d86 Server Tests run 32635355300, unit-and-api job 97184149354, failed in the transient-lock case with ERR_SQLITE_ERROR / database is locked while importing server/db.mjs.

The current one-line production repair sets SQLite PRAGMA busy_timeout = 5000 immediately after opening the process connection. GREEN: current-head Server Tests run 32635471085, unit-and-api job 97184424263, passes the same regression (legacy HTTP webhook migration regression passed) and the full unit/API lane; cloud-e2e is also terminal success. This is a causal RED→GREEN repair for short-lived writer contention, not an untested delay tweak.

The PR body’s older exact-head text is historical; use the head/base above until the body is refreshed. Hosted PR checks are still non-authorizing where they materialize the synthetic merge ref rather than the exact contributor commit, so this evidence does not relax the existing exact-head/independent-review merge gate.

[devin-ai-integration]

Devin Review found 2 new potential issues.

[seonghobae]

@jules Continue this existing branch only; do not create a competing PR and do not touch unrelated ScopeWeave lanes.

Fresh serialization point: exact current contributor head is a8fea65c328c76ca83e00a8768fcd4b3c7b6c3e6; protected live base is develop@2c328875e00e86537df3e965170be80532571cad. Server Tests run 32928850763, unit-and-api job 98057047443, checked out synthetic merge 051b82cc49d2ffb6d2cb26872558aae158d319a3 containing this exact head and fails the new realistic tests/api/fetch-boundary-ownership.test.mjs regression: importing server/app.mjs replaces caller-owned globalThis.fetch. That is current RED behavioral evidence, not merge authority.

Repair the root cause on THIS branch by replacing process-wide fetch interception with explicit, fail-closed sensitive outbound collaborators. Keep the existing SSRF/OIDC security semantics intact.

Required bounded design:

1. In server/app_core.mjs, make outbound webhook delivery and the production OIDC token exchange call explicit injected/configured security transports instead of ambient fetch. A single beginner-readable configurator is preferred if it keeps the API small.
2. Default sensitive collaborators in app_core.mjs must fail closed when the core is imported without the security facade; direct core import must not silently fall back to native fetch for webhook delivery or OIDC token exchange.
3. In server/app.mjs, configure those collaborators to reuse the existing protectedWebhookFetch and boundedOidcFetch behavior (preserve method, headers, body, signal/cancellation, timeout, Response semantics).
4. Remove the globalThis.fetch = ... installation and the Symbol guard/classification machinery that exists only to monkey-patch process fetch. Importing ScopeWeave must leave a caller-installed globalThis.fetch untouched.
5. Do not weaken public-HTTPS validation, denied address ranges, DNS revalidation/pinning, redirect rejection, POST replay rules, bounded OIDC response handling, RS256 signature/issuer/audience/azp/exp/nbf/iat/sub/nonce/email/email_verified checks, webhook signing/retry accounting, metrics, or auth/tenant behavior.
6. Keep tests/api/fetch-boundary-ownership.test.mjs as the RED→GREEN regression and add/adjust only realistic tests needed to prove direct-core sensitive calls fail closed and facade-routed webhook/OIDC flows still work. Do not turn the test into an implementation-only assertion.
7. Re-run the focused ownership regression, npm run test:api, npm run test:unit, and the applicable fuzz/security deterministic suite available locally. Report exact changed paths and commands/results here.

This also addresses the still-current architectural review observations that webhook SSRF safety depends on import order/global fetch interception. Do not resolve those threads until the exact new head is inspected and GREEN evidence exists. Preserve all unrelated long-lived #552 repairs.