Files
curriculum-project-hub/.scratch/saas-production-readiness/assets/tenant-auth-security-audit.md
T

263 lines
12 KiB
Markdown

# 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: Feishu and model-provider credentials are org-scoped secrets,
encrypted at rest, and accessed through a resolver. Provider ownership may be
Organization-managed BYOK or a distinct platform-managed connection for that
Organization; neither mode permits a process-global shared key.
## 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 erases the accepted
per-Organization provider mode and makes both BYOK and platform-managed cost
attribution 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 `<OpenRouter API key>` 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;
provider resolution honors either Organization-managed BYOK or a distinct
platform-managed connection for that Organization.
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.