Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ Playwright E2E with **server-side coverage collection** (`collectServer: true` i

## Multi-Tenancy

- `tenant_id` column on ALL tables. Every DB query MUST include an explicit tenant filter — `eq(table.tenantId, session.tenantId)` — written **per query, in the route**. There is no ORM-level or middleware-level enforcement today (`app/src/lib/db/index.ts` is a plain Drizzle client), so a forgotten filter is a cross-tenant leak that nothing catches. Adding a guard is tracked in #1226.
- `tenant_id` column on ALL tables. Every DB query MUST include an explicit tenant filter — `eq(table.tenantId, session.tenantId)` — written **per query, in the route**. There is no ORM-level or middleware-level enforcement today (`app/src/lib/db/index.ts` is a plain Drizzle client), so a forgotten filter is a cross-tenant leak that the ORM will not catch. A test-time ratchet (`app/src/lib/db/__tests__/tenant-scope.test.ts`, #1226) fails the build on any unscoped query against a tenant table — it is a safety net, not runtime enforcement, so the per-query filter is still mandatory.
- Take `tenantId` from `requireSession()`, NEVER from the request body.
- JWT tokens include `tenantId` claim. Validate before ANY DB or API access.
- SaaS vs on-prem: env vars only, never code branches.
Expand Down
21 changes: 21 additions & 0 deletions app/src/lib/__tests__/docs-accuracy.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,27 @@ describe("documentation accuracy", () => {
expect(doc).toContain("there is no `--skip-migrations` CLI flag");
});

it("CLAUDE.md points at the tenant guard by path, and that path exists", () => {
// The section used to say a forgotten tenant filter is "a leak that
// nothing catches. Adding a guard is tracked in #1226." The guard shipped
// (#1351), so that was false in a direction that changes behaviour: an
// agent reading it would either duplicate the guard or reason more
// defensively than the code requires (#1355).
//
// Pinned by PATH rather than by phrasing, so a rewrite that drops the
// pointer fails while a rewrite that keeps it is free to reword.
const doc = readDoc("CLAUDE.md");
const guardPath = "app/src/lib/db/__tests__/tenant-scope.test.ts";
expect(doc).toContain(guardPath);
expect(existsSync(resolve(REPO_ROOT, guardPath))).toBe(true);

// The mandate itself is load-bearing and must survive any rewording: the
// ratchet is a test-time safety net, NOT runtime enforcement. An agent
// that believes the ORM scopes queries will write an unscoped one.
expect(doc).toMatch(/per query, in the route/);
expect(doc).toMatch(/not runtime enforcement/);
Comment on lines +114 to +123

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Assert that the stale tracking claim stays removed.

The PR objective explicitly removes the “tracked in #1226” wording, but this test only checks that the new path and caveats exist. Reintroducing the stale claim would therefore pass CI.

Proposed assertion
     expect(doc).toMatch(/per query, in the route/);
     expect(doc).toMatch(/not runtime enforcement/);
+    expect(doc).not.toMatch(/tracked in `#1226/i`);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const doc = readDoc("CLAUDE.md");
const guardPath = "app/src/lib/db/__tests__/tenant-scope.test.ts";
expect(doc).toContain(guardPath);
expect(existsSync(resolve(REPO_ROOT, guardPath))).toBe(true);
// The mandate itself is load-bearing and must survive any rewording: the
// ratchet is a test-time safety net, NOT runtime enforcement. An agent
// that believes the ORM scopes queries will write an unscoped one.
expect(doc).toMatch(/per query, in the route/);
expect(doc).toMatch(/not runtime enforcement/);
const doc = readDoc("CLAUDE.md");
const guardPath = "app/src/lib/db/__tests__/tenant-scope.test.ts";
expect(doc).toContain(guardPath);
expect(existsSync(resolve(REPO_ROOT, guardPath))).toBe(true);
// The mandate itself is load-bearing and must survive any rewording: the
// ratchet is a test-time safety net, NOT runtime enforcement. An agent
// that believes the ORM scopes queries will write an unscoped one.
expect(doc).toMatch(/per query, in the route/);
expect(doc).toMatch(/not runtime enforcement/);
expect(doc).not.toMatch(/tracked in `#1226/i`);
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/src/lib/__tests__/docs-accuracy.test.ts` around lines 114 - 123, Update
the documentation accuracy test around the existing CLAUDE.md assertions to
explicitly verify that the stale “tracked in `#1226`” wording is absent, while
preserving the current path and caveat checks.

});

it("the deploy skill does not send auditors looking for a flag that does not exist", () => {
// CLAUDE.md was corrected but the deploy skill still listed
// "`--skip-migrations` flag missing or undocumented" as a gap to capture
Expand Down
Loading