Files
PaperClipAI/server/src/__tests__/multilingual-issues-routes.test.ts
T
Jannes StubbemannandClaude Opus 4.8 606e74d11f cloud_tenant: company-scoped tenants, never instance-admin (#7525)
## Thinking Path

> - Paperclip orchestrates AI agents for zero-human companies, and a
single server instance can host many companies.
> - The auth middleware (`server/src/middleware/auth.ts`) supports a
`cloud_tenant` mode where a trusted hosting proxy injects per-request
identity headers, designed originally for one-deployment-per-tenant
setups.
> - In that original setup, granting every cloud tenant the
`instance_admin` role was harmless; on a **shared, multi-tenant pool**
it means any paying tenant is admin of the whole instance and can reach
every other tenant's data.
> - A tenant only needs to own its own company — which it already gets
via the company membership the same code path upserts — so
instance-level admin is never appropriate for `cloud_tenant` actors.
> - This PR removes the `instance_admin` grant from the cloud-tenant
path and pins `isInstanceAdmin: false` on the resolved actor.
> - Greptile review then surfaced a follow-up gap: deployments that ran
the pre-hardening build still have stale `instance_admin` rows in
`instance_user_roles`, which other lookups (BetterAuth session path,
board API keys, and the authorization service's own DB re-check) would
still honor.
> - The follow-up commit closes that gap by purging stale rows at the
cloud-tenant auth boundary and by teaching the authorization service
that `cloud_tenant` actors are never instance admins.
> - The benefit is that shared-pool hosting becomes structurally safe:
tenants are company-scoped owners, never instance admins — including on
deployments upgrading from the older behavior.

## Linked Issues

- Refs #966 — managed SaaS multi-tenant hosting is the deployment shape
this hardening protects.
- Refs #5015 — same problem space: instance-admin-scoped credentials are
too broad for multi-company instances; tenants need company-scoped
access.

Neither issue is fully closed by this PR; it removes the instance-admin
grant from the `cloud_tenant` trusted-header path specifically.

## What Changed

- `server/src/middleware/auth.ts`
- Removed the `instanceUserRoles` insert that granted every cloud tenant
`instance_admin`; `resolveCloudTenantActor` now returns
`isInstanceAdmin: false` (was `true`).
- `resolveCloudTenantActor` now **deletes** any stale `instance_admin`
row for the authenticated tenant user on every trusted-header request,
so grants left behind by pre-hardening deployments are purged at the
source (closes the Greptile P2: stale rows could otherwise re-elevate
the user via the BetterAuth session path, board API keys, or the
authorization service).
  - The function is `export`ed so it can be unit-tested directly.
- `server/src/services/authorization.ts`
- `authorizationService` previously re-checked `instanceUserRoles` from
the DB regardless of the actor flag, which would have elevated even
hardened `cloud_tenant` actors while a stale row lingered. Actors with
`source === "cloud_tenant"` are now never elevated to instance admin;
other board actors keep the existing lookup.
- `server/src/services/authorization.ts` +
`server/src/middleware/auth.ts` (follow-up commit `dc57a71c7`)
- CI on the merge ref surfaced that elevation removal alone strands real
cloud tenant users: board actors only ever reached `issue:read` /
`issue:mutate` through instance-admin elevation (`permissionForAction`
maps both to no grant key). `decide()` now grants `cloud_tenant` actors
with an **active membership in the resource company** the same read
surface as a same-company agent (`agent:read`, `company_scope:read`,
`issue:read`, `project:read`) plus `issue:mutate` for non-viewer members
— cross-company access stays denied (new `allow_company_member` reason).
- `resolveCloudTenantActor` seeds the standard role-default permission
grants (`ensureHumanRoleDefaultGrants`) so granted actions (e.g.
`tasks:assign`, `agents:create` for owners) work without elevation.
- Master-side route tests that stubbed cloud tenant actors with
`isInstanceAdmin: true` now seed a real membership and assert under the
hardened contract (`issue-identifier-routes`,
`multilingual-issues-routes`, `issue-comment-redaction`).
- Tests
- `server/src/middleware/cloud-tenant-actor.test.ts` (new): cloud tenant
is never instance-admin, is scoped to exactly the one company from its
stack, still upserts user/company/membership, purges stale
`instance_admin` rows, returns null without the server token, and maps
non-owner stack roles without elevating.
- `server/src/__tests__/auth-session-route.test.ts`: end-to-end
middleware regression — a user with a stale `instance_admin` row stops
being elevated via the session path once they authenticate through the
cloud-tenant path (with a control assertion showing the pre-purge
elevation).
- `server/src/__tests__/authorization-service.test.ts` (embedded
Postgres): a `cloud_tenant` actor with a stale `instance_admin` row in
the real DB cannot cross company boundaries, while a `session` actor
with the same row still resolves `allow_instance_admin`.

## Verification

Run from the repo root after `pnpm install --frozen-lockfile`:

```bash
cd server
npx vitest run src/middleware/cloud-tenant-actor.test.ts src/__tests__/auth-session-route.test.ts
# 9 tests passed
npx vitest run src/__tests__/authorization-service.test.ts
# 16 tests passed (embedded Postgres)
pnpm typecheck
# clean
```

Also ran the broader auth-related suites locally (`auth-routes`,
`authz-company-access`, `better-auth`, `adapter-routes-authz`,
`express5-auth-wildcard`): 8 files, 58 tests, all passing.

## Risks

- **This touches authentication and authorization paths directly.**
Mistakes here are security bugs in both directions; review accordingly.
- **Behavioral change for existing `cloud_tenant` deployments:** tenants
that previously (incorrectly) had instance-admin lose it — including the
ability to see/manage other companies on the instance. This is the
intended hardening, but any single-tenant deployment that relied on the
cloud-tenant identity for instance administration must provision a
separate admin identity.
- **The purge is destructive by design:** if an operator's
instance-admin identity is *also* provisioned through the cloud-tenant
headers (same user id), its `instance_admin` row will be deleted on the
next trusted-header request. Operators should hold admin through a
non-cloud-tenant identity.
- **Residual gap (documented, not fixed here):** a deployment that ran
the old cloud_tenant build and then *disabled* cloud-tenant mode keeps
stale rows until the affected user re-authenticates through the cloud
path. A data migration was considered and deliberately avoided: there is
no reliable SQL predicate for "cloud-tenant-provisioned user" (no source
column), so a migration risks deleting legitimate admins.
- No schema or migration changes; no UI changes.

## Model Used

- Claude Fable 5 (claude-fable-5, 1M context), extended thinking + tool
use, via Claude Code — this revision; original PR authored in an earlier
Claude Code session.

## 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 (none duplicate this; related issues Refs #966 / #5015 are
linked in the issue section)
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] If this change affects the UI, I have included before/after
screenshots (no UI changes)
- [x] I have updated relevant documentation to reflect my changes (no
existing docs reference `cloud_tenant` mode)
- [x] I have considered and documented any risks above
- [x] I will address all Greptile and reviewer comments before
requesting merge

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-11 17:59:32 -07:00

193 lines
7.0 KiB
TypeScript

import { randomUUID } from "node:crypto";
import express from "express";
import request from "supertest";
import { afterAll, beforeAll, describe, expect, it, vi } from "vitest";
import { companies, companyMemberships, createDb } from "@paperclipai/db";
import {
getEmbeddedPostgresTestSupport,
startEmbeddedPostgresTestDatabase,
} from "./helpers/embedded-postgres.js";
import { errorHandler } from "../middleware/index.js";
import { issueRoutes } from "../routes/issues.js";
import type { StorageService } from "../storage/types.js";
const embeddedPostgresSupport = await getEmbeddedPostgresTestSupport();
const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe.sequential : describe.skip;
if (!embeddedPostgresSupport.supported) {
console.warn(
`Skipping embedded Postgres multilingual issue route tests on this host: ${embeddedPostgresSupport.reason ?? "unsupported environment"}`,
);
}
describeEmbeddedPostgres("multilingual issue routes", () => {
let db!: ReturnType<typeof createDb>;
let tempDb: Awaited<ReturnType<typeof startEmbeddedPostgresTestDatabase>> | null = null;
let app!: ReturnType<typeof createApp>;
let companyId!: string;
const title = "验证中文任务";
const description = [
"请用中文回复并保留上下文。",
"日本語: 次の手順を書いてください。",
"हिन्दी: कृपया स्थिति बताएं।",
].join("\n");
const firstReply = [
"结果: 中文响应保留。",
"日本語の返信も保持。",
"हिन्दी उत्तर भी सुरक्षित है।",
].join("\n");
const completionNote = [
"完成: 已验证中文。",
"日本語: 完了しました。",
"हिन्दी: सत्यापन पूरा हुआ।",
].join("\n");
const documentBody = [
"# QA notes",
"",
"- 中文: 可以创建、读取、搜索、评论。",
"- 日本語: ドキュメント本文を保持します。",
"- हिन्दी: दस्तावेज़ पाठ सुरक्षित रहता है।",
].join("\n");
beforeAll(async () => {
tempDb = await startEmbeddedPostgresTestDatabase("paperclip-multilingual-issues-");
db = createDb(tempDb.connectionString);
companyId = randomUUID();
app = createApp(companyId);
await db.insert(companies).values({
id: companyId,
name: "Multilingual tenant",
issuePrefix: "LNG",
requireBoardApprovalForNewAgents: false,
});
await db.insert(companyMemberships).values({
companyId,
principalType: "user",
principalId: "cloud-user-1",
status: "active",
membershipRole: "owner",
updatedAt: new Date(),
});
}, 20_000);
afterAll(async () => {
await tempDb?.cleanup();
});
function createStorage(): StorageService {
return {
provider: "local_disk",
putFile: vi.fn(async () => {
throw new Error("Unexpected storage.putFile call in multilingual issue route test");
}),
getObject: vi.fn(async () => {
throw new Error("Unexpected storage.getObject call in multilingual issue route test");
}),
headObject: vi.fn(async () => ({ exists: false })),
deleteObject: vi.fn(async () => undefined),
};
}
function createApp(companyId: string) {
const app = express();
app.use(express.json());
app.use((req, _res, next) => {
(req as any).actor = {
type: "board",
userId: "cloud-user-1",
companyIds: [companyId],
memberships: [{ companyId, membershipRole: "owner", status: "active" }],
source: "cloud_tenant",
// cloud_tenant actors are never instance admins — reads flow through
// the active company membership seeded in beforeAll.
isInstanceAdmin: false,
};
next();
});
app.use("/api", issueRoutes(db, createStorage()));
app.use(errorHandler);
return app;
}
it("creates an issue with multilingual title and description", async () => {
const createRes = await request(app)
.post(`/api/companies/${companyId}/issues`)
.send({
title,
description,
status: "todo",
priority: "medium",
});
expect(createRes.status, JSON.stringify(createRes.body)).toBe(201);
expect(createRes.body).toMatchObject({
title,
description,
status: "todo",
priority: "medium",
identifier: "LNG-1",
});
});
it("reads the multilingual title and description unchanged", async () => {
const getRes = await request(app).get("/api/issues/LNG-1");
expect(getRes.status, JSON.stringify(getRes.body)).toBe(200);
expect(getRes.body.title).toBe(title);
expect(getRes.body.description).toBe(description);
});
it("finds the issue by Chinese search text", async () => {
const searchRes = await request(app).get(`/api/companies/${companyId}/issues`).query({ q: "中文" });
expect(searchRes.status, JSON.stringify(searchRes.body)).toBe(200);
expect(searchRes.body.map((issue: { identifier: string }) => issue.identifier)).toContain("LNG-1");
});
it("preserves multilingual comment bodies", async () => {
const commentRes = await request(app)
.post("/api/issues/LNG-1/comments")
.send({ body: firstReply });
expect(commentRes.status, JSON.stringify(commentRes.body)).toBe(201);
expect(commentRes.body.body).toBe(firstReply);
});
it("preserves multilingual document bodies", async () => {
const documentRes = await request(app)
.put("/api/issues/LNG-1/documents/qa-notes")
.send({
title: "Multilingual QA",
format: "markdown",
body: documentBody,
});
expect(documentRes.status, JSON.stringify(documentRes.body)).toBe(201);
expect(documentRes.body.body).toBe(documentBody);
});
it("preserves multilingual completion comments", async () => {
const completeRes = await request(app)
.patch("/api/issues/LNG-1")
.send({ status: "done", comment: completionNote });
expect(completeRes.status, JSON.stringify(completeRes.body)).toBe(200);
expect(completeRes.body.status).toBe("done");
expect(completeRes.body.comment.body).toBe(completionNote);
});
it("lists multilingual comments in write order", async () => {
const commentsRes = await request(app).get("/api/issues/LNG-1/comments").query({ order: "asc" });
expect(commentsRes.status, JSON.stringify(commentsRes.body)).toBe(200);
expect(commentsRes.body.map((comment: { body: string }) => comment.body)).toEqual([
firstReply,
completionNote,
]);
});
it("exposes multilingual issue text in heartbeat context", async () => {
const heartbeatContextRes = await request(app).get("/api/issues/LNG-1/heartbeat-context");
expect(heartbeatContextRes.status, JSON.stringify(heartbeatContextRes.body)).toBe(200);
expect(heartbeatContextRes.body.issue.title).toBe(title);
expect(heartbeatContextRes.body.issue.description).toBe(description);
expect(heartbeatContextRes.body.commentCursor.totalComments).toBe(2);
});
});