From 682bc70563d2d455fc1d76d539312ff3635f81f4 Mon Sep 17 00:00:00 2001 From: Hong Jiarong Date: Fri, 10 Jul 2026 03:05:58 +0800 Subject: [PATCH] docs: resolve tenant security audit --- .../assets/tenant-auth-security-audit.md | 258 ++++++++++++++++++ .../issues/02-audit-tenant-auth-security.md | 30 +- .../08-prove-production-release-gate.md | 2 +- ...6-confine-agent-and-protect-credentials.md | 12 + .../17-close-mcp-data-egress-escapes.md | 11 + ...8-bind-and-reauthorize-deferred-actions.md | 11 + ...-design-org-scoped-secret-control-plane.md | 12 + ...-design-production-web-session-boundary.md | 11 + ...21-clear-dependency-security-advisories.md | 11 + .../issues/22-enforce-organization-status.md | 11 + .scratch/saas-production-readiness/map.md | 1 + 11 files changed, 368 insertions(+), 2 deletions(-) create mode 100644 .scratch/saas-production-readiness/assets/tenant-auth-security-audit.md create mode 100644 .scratch/saas-production-readiness/issues/16-confine-agent-and-protect-credentials.md create mode 100644 .scratch/saas-production-readiness/issues/17-close-mcp-data-egress-escapes.md create mode 100644 .scratch/saas-production-readiness/issues/18-bind-and-reauthorize-deferred-actions.md create mode 100644 .scratch/saas-production-readiness/issues/19-design-org-scoped-secret-control-plane.md create mode 100644 .scratch/saas-production-readiness/issues/20-design-production-web-session-boundary.md create mode 100644 .scratch/saas-production-readiness/issues/21-clear-dependency-security-advisories.md create mode 100644 .scratch/saas-production-readiness/issues/22-enforce-organization-status.md diff --git a/.scratch/saas-production-readiness/assets/tenant-auth-security-audit.md b/.scratch/saas-production-readiness/assets/tenant-auth-security-audit.md new file mode 100644 index 0000000..b5e8a94 --- /dev/null +++ b/.scratch/saas-production-readiness/assets/tenant-auth-security-audit.md @@ -0,0 +1,258 @@ +# Tenant, authentication, and request security audit + +## Verdict + +The current Hub has a sound application-level foundation for ordinary org-admin +CRUD isolation, but it is **not safe to deploy as a multi-tenant SaaS**. Several +agent and asynchronous-action paths can cross the Organization/Project boundary, +and the current process-global credential design directly contradicts the +org-scoped encrypted-secret decision in ADR-0021. + +The answer to the audit question is therefore **yes**: current filesystem, +Feishu-context, deferred authorization, card-action, and credential paths can +cross or bypass the intended boundary. These are implementation divergences, +not unspecified product behavior. + +## Contract baseline + +The audit used these upstream requirements: + +- `Spec.System.Organization`: PROJECT and TEAM each have one Organization; + TEAM→PROJECT grants are same-org only. +- `Spec.System.ProjectWorkspace`: folders are transparent; folder/project + placement must remain inside one Organization. +- `Spec.System.Permission`: agent trigger requires EDIT, normal cancel requires + MANAGE, and force release is outside the project role lattice. +- `Spec.System.Memory`: a run may read Feishu context only from its project's + bound chat. +- `Spec.System.AgentSurface`: every agent file operation must stay inside that + run's project workspace. +- ADR-0019: authorization is evaluated against principals resolved at request + time so membership changes take effect immediately. +- ADR-0021: customer Feishu and model-provider credentials are org-scoped + secrets, encrypted at rest, and accessed through a resolver. + +## Confirmed protections + +The following paths are correctly fail-closed today: + +- Every `/api/org/:orgSlug/*` route calls `requireOrgRole`; the guard reloads + the user, active membership, allowed OWNER/ADMIN role, and ACTIVE org for each + request. +- Project/session/folder/team service functions compare the requested object's + Organization before returning or mutating it. Cross-org lookups return not + found instead of revealing the foreign object. +- `PermissionAuthorizer` derives the Organization from the resource before + expanding team and external principals. `grantTeamProjectAccess` refuses a + TEAM whose Organization differs from the Project and detects corrupt active + grants when listing. +- Feishu messages with no sender `open_id` are denied. Bound-chat triggers run + through `agent.trigger` and `role.trigger` gates before creating an AgentRun. +- OAuth state is signed, expiry-checked, and paired with an HttpOnly nonce + cookie. `returnTo` is constrained to same-origin `/admin` paths. Session + signatures use HMAC-SHA256 with constant-time comparison, and session users + are reloaded on every request. +- Fastify's current default body limit is 1 MiB, so HTTP bodies are not + completely unbounded even though field-level and rate limits are absent. + +Executed evidence: + +```text +5 integration files passed +28 tests passed +``` + +Those tests include unauthenticated access, member-vs-admin roles, cross-org +project IDs, Organization-scoped team slugs, and cross-org TEAM grant refusal. + +## Critical findings + +### 1. The agent receives service secrets and can read other tenant data + +`hub/src/agent/runner.ts` builds the SDK environment as: + +```text +{ ...process.env, ...providerEnv } +``` + +The sandboxed agent and its Bash subprocess therefore inherit `DATABASE_URL`, +`FEISHU_APP_SECRET`, `HUB_SESSION_SECRET`, the model-provider token, and every +other service variable. A model or prompt injection can run `env`; network +egress is intentionally open, so exposed values can leave the host. + +The same sandbox config has only `allowWrite: [workspaceDir]`. Reads outside +the workspace are allowed except for a small deny list. All organizations run +as the same service user, and sibling project workspaces are not denied. This +violates `AgentFileOp.Authorized`, which covers reads as well as writes. + +The installed Claude Agent SDK already exposes two mechanisms that are not +used: sandbox credential rules can `deny` or proxy-`mask` environment variables, +and filesystem rules can deny a workspace root while re-allowing the current +workspace. Production still needs an actual Linux test proving that SDK/Bash +cannot read another project or service credential. + +### 2. MCP file and Feishu-context tools escape their run objects + +`resolveDeliverableFile` intentionally adds the repository Git root to allowed +roots, even when it is above the project workspace. It also checks lexical paths +with `stat`, which follows symlinks. A deterministic reproduction created +`workspace/leak.txt` as a symlink to another tenant's file: + +```json +{"symlinkEscapeAccepted":true,"outsideContentReadable":true} +``` + +The checked-in unit test currently treats sending a repo-root `README.md` from +a nested project as desired behavior. That expectation is a direct divergence +from the AgentSurface contract and must be reversed. + +`readFeishuContext` ignores its `ToolContext`. The MCP wrapper closes over the +bound `chatId`, but the model supplies an arbitrary message/thread ID and the +reader never verifies the returned message's `chat_id`. A stub Feishu API +returning a message from `chat-other` produced: + +```json +{"returnedChatId":"chat-other","crossChatContentReturned":true} +``` + +The same validation is required for thread parents and every returned item. + +Inbound downloads have the inverse symlink problem: the unsandboxed Hub writes +to `workspace/.cph/inbox`. A prior agent command can replace a path component +with a symlink, turning the Hub into a confused deputy unless writes use a +verified real path/no-follow boundary. + +### 3. SaaS credentials are process-global instead of org-scoped + +`server.ts` creates one process-global Feishu client/listener and passes one +global app ID/secret to OAuth. `feishuOAuth.ts` explicitly calls per-org +encrypted credentials "deferred", although ADR-0021 says they **must** be +org-scoped, encrypted at rest, and accessed through a resolver. + +`EnvRuntimeSettings.provider` similarly discards its `scope` argument and uses +one global OpenRouter token for every project. This both leaks tenant ownership +of credentials and makes the accepted "customers supply their own provider +keys" accounting model false. The schema has no encrypted credential store, +key version, rotation state, or resolver seam. + +This is also a product reachability blocker: one customer-owned Feishu app +cannot receive bot/OAuth traffic for unrelated customer tenants under the +ADR-0021 model. + +## High findings + +### 4. Delayed and card actions are not bound to the authorized object + +- `onMessage` checks `agent.trigger` before enqueueing/batching, but + `startAgentRun` checks only `role.trigger`. If a grant, team membership, or + tenant status changes while a request waits, it still starts. This contradicts + ADR-0019's request-time effectiveness rule. +- Interrupt handling authorizes `agent.cancel` against the project bound to the + card action's current chat, then calls `activeRuns.get(runId)` without proving + that `runId` belongs to that project. A stale, forwarded, or otherwise + mismatched action can abort another project's active controller. +- Approval handling stores the expected `chatId` but never compares it to the + action context before resolving the pending approval. + +Execution-time authorization must resolve one immutable tuple +`(organization, project, run/action, actor, chat)` and reject any mismatch +before mutation. + +### 5. SUSPENDED organizations can still execute bound projects + +HTTP org-admin guards and unbound-chat onboarding reject non-ACTIVE orgs, but +`PermissionAuthorizer.organizationForResource` returns only `organizationId` +and never checks status. A real database reproduction created a SUSPENDED org, +project, and EDIT grant: + +```json +{"organizationStatus":"SUSPENDED","agentTriggerAllowed":true} +``` + +Project onboarding helpers also check membership existence without checking +Organization status. The same status must be enforced at the shared resource +authorization/onboarding seam, not independently in selected transports. + +### 6. Production dependency graphs contain known vulnerabilities + +`npm audit --omit=dev` exits 1 with two high findings: + +- `@larksuiteoapi/node-sdk@1.68.0` +- transitive `axios@1.13.6` + +The current latest Feishu SDK (`1.70.0` at audit time) still pins Axios +`~1.13.3`, while the audited fixed Axios line is newer. The relevant advisories +include request/credential manipulation, proxy credential leakage, ReDoS, and +unbounded resource allocation; for example +[GHSA-3g43-6gmg-66jw](https://github.com/advisories/GHSA-3g43-6gmg-66jw), +[GHSA-hfxv-24rg-xrqf](https://github.com/advisories/GHSA-hfxv-24rg-xrqf), and +[GHSA-777c-7fjr-54vf](https://github.com/advisories/GHSA-777c-7fjr-54vf). +An override is outside the SDK's declared semver range and therefore requires +integration testing rather than a blind lockfile edit. + +`cargo audit` exits 1 with five vulnerability records: + +- `crossbeam-epoch@0.9.18`, patched in `>=0.9.20`; +- `quick-xml@0.38.4` and `0.39.4`, each affected by namespace-allocation and + quadratic-attribute DoS advisories, patched in `>=0.41.0`. + +It also reports `memmap2@0.9.10` as unsound (patched in `>=0.9.11`) and four +unmaintained transitive crates. All enter through the Typst 0.15 dependency +graph. References: +[RUSTSEC-2026-0204](https://rustsec.org/advisories/RUSTSEC-2026-0204.html), +[RUSTSEC-2026-0194](https://rustsec.org/advisories/RUSTSEC-2026-0194.html), +[RUSTSEC-2026-0195](https://rustsec.org/advisories/RUSTSEC-2026-0195.html), and +[RUSTSEC-2026-0186](https://rustsec.org/advisories/RUSTSEC-2026-0186.html). + +No `npm audit` or `cargo audit` gate exists in CI. + +## Browser/session and availability gaps + +These do not currently prove a cross-org data read, but they are required before +exposing the service publicly: + +- state-changing cookie-auth routes have no CSRF token or Origin/Sec-Fetch-Site + enforcement; SameSite=Lax is the only browser-side barrier; +- there is no session revocation/version store, so a stolen cookie remains valid + for up to seven days unless the global secret rotates; +- startup accepts any non-empty session secret rather than enforcing generated + entropy, and there is no key rotation scheme; +- no security-header policy is registered, and Fastify request timeout is zero; +- OAuth, login, and org APIs have no per-IP/user/tenant rate limit; +- Feishu file/post download count, response size, MIME, archive shape, and total + workspace quota are unbounded; uploads read entire files into memory. + +The exact abuse quotas and file limits remain with +`Define initial abuse and capacity controls`; they must not be guessed here. + +## Secret scan + +All 377 tracked files were scanned for private-key blocks, common cloud/API-key +shapes, and non-empty assignments to the Hub's principal secret variables. No +private key or real credential was found. The only assignment hits were the +documented `` placeholder and local/test PostgreSQL URLs. +`hub/.env` is ignored and untracked. + +This is positive repository hygiene, but it does not mitigate the runtime +environment leak into the agent process. + +## Required security release evidence + +Production readiness requires automated proof of all of the following: + +1. An agent/Bash process cannot read service credentials, another project + workspace, repo-root files, or a symlink target outside its workspace. +2. Feishu context and file delivery validate the real target after resolution, + not only a caller-supplied lexical ID/path. +3. Every delayed action reauthorizes at execution and binds actor, chat, + organization, project, run, and pending action consistently. +4. ACTIVE Organization status is enforced in the shared authorization seam. +5. Feishu and provider credentials resolve by Organization, are encrypted at + rest with versioned keys, never enter logs/agent tools, and can rotate. +6. Cross-org HTTP and Prisma service tests remain green, while new negative + tests cover every finding above. +7. Production Node and Rust dependency audits pass with no ignored + vulnerability/unsound advisory unless a time-bounded, reviewed exception is + explicitly recorded. +8. Browser/session, request, and file-ingestion limits are verified against the + accepted production threat and capacity policy. diff --git a/.scratch/saas-production-readiness/issues/02-audit-tenant-auth-security.md b/.scratch/saas-production-readiness/issues/02-audit-tenant-auth-security.md index ea826ff..75af0e3 100644 --- a/.scratch/saas-production-readiness/issues/02-audit-tenant-auth-security.md +++ b/.scratch/saas-production-readiness/issues/02-audit-tenant-auth-security.md @@ -1,7 +1,7 @@ # Audit tenant, authentication, and request security boundaries Type: research -Status: open +Status: resolved ## Question @@ -10,3 +10,31 @@ Organization boundary, bypass the intended platform/org/project authorization seams, expose secrets, or accept unbounded hostile input in the initial production topology? +## Answer + +Yes. Basic org-admin HTTP/Prisma scoping is correctly enforced and its 28 +targeted integration tests pass, but the agent inherits service secrets and can +read sibling files, MCP file/context tools accept real targets outside the run +objects, queued/card actions do not consistently bind or reauthorize their +target, non-ACTIVE tenants can still trigger agents, and Feishu/provider +credentials remain global instead of ADR-0021 org-scoped encrypted secrets. +Current Node and Rust dependency audits also fail on high/vulnerability/unsound +advisories. + +The complete contract mapping, reproductions, positive evidence, advisory +inventory, and required release proofs are in the +[tenant, authentication, and request security audit](../assets/tenant-auth-security-audit.md). + +The newly-clear implementation and decision frontier is: + +- [Confine the agent runtime and protect service credentials](16-confine-agent-and-protect-credentials.md) +- [Close MCP context and file-delivery escape paths](17-close-mcp-data-egress-escapes.md) +- [Bind and reauthorize deferred and card actions](18-bind-and-reauthorize-deferred-actions.md) +- [Design the org-scoped secret and connection control plane](19-design-org-scoped-secret-control-plane.md) +- [Design the production browser and session boundary](20-design-production-web-session-boundary.md) +- [Clear dependency advisories and gate releases](21-clear-dependency-security-advisories.md) +- [Enforce Organization status at the shared authorization seam](22-enforce-organization-status.md) + +Inbound Feishu file count/size/MIME and request-rate policy remain owned by +[Define initial abuse and capacity controls](05-define-abuse-capacity-controls.md) +rather than being guessed in this audit. diff --git a/.scratch/saas-production-readiness/issues/08-prove-production-release-gate.md b/.scratch/saas-production-readiness/issues/08-prove-production-release-gate.md index 4dfe3cc..bc223b9 100644 --- a/.scratch/saas-production-readiness/issues/08-prove-production-release-gate.md +++ b/.scratch/saas-production-readiness/issues/08-prove-production-release-gate.md @@ -2,7 +2,7 @@ Type: task Status: open -Blocked by: 01, 02, 03, 04, 05, 06, 07, 09, 10, 11, 12, 13, 14, 15 +Blocked by: 01, 02, 03, 04, 05, 06, 07, 09, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22 ## Question diff --git a/.scratch/saas-production-readiness/issues/16-confine-agent-and-protect-credentials.md b/.scratch/saas-production-readiness/issues/16-confine-agent-and-protect-credentials.md new file mode 100644 index 0000000..b36ea07 --- /dev/null +++ b/.scratch/saas-production-readiness/issues/16-confine-agent-and-protect-credentials.md @@ -0,0 +1,12 @@ +# Confine the agent runtime and protect service credentials + +Type: task +Status: open + +## Question + +Make the real Claude SDK/Bash execution surface uphold `AgentFileOp.Authorized`: +deny sibling tenant workspaces and service files, expose only the minimum safe +process environment, use SDK credential protection for secrets, and add an +actual Linux sandbox test proving another workspace and every non-provider +credential are unreadable while required tools still work. diff --git a/.scratch/saas-production-readiness/issues/17-close-mcp-data-egress-escapes.md b/.scratch/saas-production-readiness/issues/17-close-mcp-data-egress-escapes.md new file mode 100644 index 0000000..8a3abf7 --- /dev/null +++ b/.scratch/saas-production-readiness/issues/17-close-mcp-data-egress-escapes.md @@ -0,0 +1,11 @@ +# Close MCP context and file-delivery escape paths + +Type: task +Status: open + +## Question + +Restrict file delivery to a realpath/no-follow target inside the current project +workspace, remove Git-root access, make inbound Hub writes symlink-safe, and +validate every Feishu message/thread result against the run's bound chat before +returning data to the model. diff --git a/.scratch/saas-production-readiness/issues/18-bind-and-reauthorize-deferred-actions.md b/.scratch/saas-production-readiness/issues/18-bind-and-reauthorize-deferred-actions.md new file mode 100644 index 0000000..9a2c4b7 --- /dev/null +++ b/.scratch/saas-production-readiness/issues/18-bind-and-reauthorize-deferred-actions.md @@ -0,0 +1,11 @@ +# Bind and reauthorize deferred and card actions + +Type: task +Status: open + +## Question + +At execution time, reauthorize queued/batched triggers and validate one +actor/chat/organization/project/run-or-pending-action tuple for interrupt, +approval, and onboarding card actions so revoked permissions and mismatched +object IDs cannot affect another project. diff --git a/.scratch/saas-production-readiness/issues/19-design-org-scoped-secret-control-plane.md b/.scratch/saas-production-readiness/issues/19-design-org-scoped-secret-control-plane.md new file mode 100644 index 0000000..a71ac2d --- /dev/null +++ b/.scratch/saas-production-readiness/issues/19-design-org-scoped-secret-control-plane.md @@ -0,0 +1,12 @@ +# Design the org-scoped secret and connection control plane + +Type: grilling +Status: open + +## Question + +Which key-management, envelope-encryption, rotation, connection identity, and +runtime resolver design will implement ADR-0021's org-scoped Feishu and model +provider credentials, support one customer-owned Feishu app per tenant, and +ensure plaintext credentials never reach business records, logs, or agent +tools? diff --git a/.scratch/saas-production-readiness/issues/20-design-production-web-session-boundary.md b/.scratch/saas-production-readiness/issues/20-design-production-web-session-boundary.md new file mode 100644 index 0000000..76b1af3 --- /dev/null +++ b/.scratch/saas-production-readiness/issues/20-design-production-web-session-boundary.md @@ -0,0 +1,11 @@ +# Design the production browser and session boundary + +Type: grilling +Status: open + +## Question + +What same-origin/CSRF, proxy/TLS, security-header, session lifetime, +revocation, secret rotation, and request-timeout contract should the initial +org-admin web surface enforce, and which parts require server-side session state +rather than the current seven-day stateless cookie? diff --git a/.scratch/saas-production-readiness/issues/21-clear-dependency-security-advisories.md b/.scratch/saas-production-readiness/issues/21-clear-dependency-security-advisories.md new file mode 100644 index 0000000..b088893 --- /dev/null +++ b/.scratch/saas-production-readiness/issues/21-clear-dependency-security-advisories.md @@ -0,0 +1,11 @@ +# Clear dependency advisories and gate releases + +Type: task +Status: open + +## Question + +Remove every current `npm audit --omit=dev` high finding and every `cargo audit` +vulnerability/unsound finding through tested upgrades, compatible overrides or +upstream fixes—not ignores—and add reproducible Node and Rust security-audit +gates to CI and the production release evidence. diff --git a/.scratch/saas-production-readiness/issues/22-enforce-organization-status.md b/.scratch/saas-production-readiness/issues/22-enforce-organization-status.md new file mode 100644 index 0000000..0942f5b --- /dev/null +++ b/.scratch/saas-production-readiness/issues/22-enforce-organization-status.md @@ -0,0 +1,11 @@ +# Enforce Organization status at the shared authorization seam + +Type: task +Status: open + +## Question + +Make non-ACTIVE Organizations fail closed for project authorization, +onboarding, queued execution, and Feishu triggers through one shared seam, with +negative tests proving a SUSPENDED or ARCHIVED tenant cannot create, bind, +trigger, resume, or mutate work while operator recovery remains explicit. diff --git a/.scratch/saas-production-readiness/map.md b/.scratch/saas-production-readiness/map.md index 06a0bf1..9d5ebee 100644 --- a/.scratch/saas-production-readiness/map.md +++ b/.scratch/saas-production-readiness/map.md @@ -29,6 +29,7 @@ recovery procedures, and no known critical security or data-integrity gaps. ## Decisions so far - [Audit the clean-host deployment and rollback contract](issues/01-audit-clean-host-deployment.md) — keep the accepted single-host topology, but replace the incomplete in-place updater with a provisioned, immutable, readiness-gated and rollbackable release contract. +- [Audit tenant, authentication, and request security boundaries](issues/02-audit-tenant-auth-security.md) — retain the working org-admin/application authorization core, but production is blocked on agent/MCP isolation, execution-time object binding, org-scoped encrypted credentials, tenant status, browser sessions, and clean dependency audits. ## Fog