RLS: support the explicit table name APIs

[Nicolapps]

---

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Summary by CodeRabbit

• New Features

  • Added support for explicitly specifying table names in row-level security operations while maintaining backward compatibility with implicit table naming.

• Tests

  • Expanded test coverage to validate access control behavior across implicit and explicit table naming scenarios and default policy behaviors.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

The changes enhance Row Level Security functionality by introducing explicit table-name parameters across read/write operations. Both the implementation and tests are updated to support implicit (single-argument) and explicit (table-qualified) API patterns, with test coverage expanded for both calling conventions.

Changes

Cohort / File(s)	Summary
RLS Implementation Overloads packages/convex-helpers/server/rowLevelSecurity.ts	Added table-aware overloads for WrapReader.get and WrapWriter methods (get, patch, replace, delete). Updated checkAuth signature to accept optional tableNameArg parameter. Introduced NonUnion type alias for generic constraint. Added WithOptionalSystemFields import from convex/server.
RLS Test Expansion packages/convex-helpers/server/rowLevelSecurity.test.ts	Reorganized single deletion test into describe block with separate "implicit table names" and "explicit table names" test cases. Expanded default-policy suite similarly with parallel implicit and explicit variants. Added ts-expect-error annotations and new deletion/patching code paths using explicit table-name API.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Type system complexity: Multiple overloads with generic constraints and the new NonUnion type require careful verification of type inference
• Table-name resolution logic: Ensure checkAuth and operation methods correctly handle both implicit (derive from id) and explicit (from table parameter) table-name resolution paths
• Test coverage: Verify that both implicit and explicit calling patterns are exercised with proper permission assertions

Poem

🐰 A table by any name still needs a guard,
Whether whispered soft or stated plain and hard,
Explicit paths now join the implicit way,
Two routes to access, both secure to stay! 🔐

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: adding support for explicit table name APIs in the row-level security (RLS) module, which aligns with the actual implementation changes.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch nicolas/explicit-table-names-rls

---

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

[pkg-pr-new]

Open in StackBlitz

npm i https://pkg.pr.new/get-convex/convex-helpers@873

commit: 6896b08

[coderabbitai]

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

packages/convex-helpers/server/rowLevelSecurity.ts (1)

239-257: Fix security bypass in get(id) when defaultPolicy: "deny" is configured.

The single-argument get(id) form bypasses defaultPolicy: "deny" for tables without explicit rules. When an ID belongs to a table not in the rules configuration, tableName() returns null (line 220), causing the condition on line 251 (if (tableName && ...)) to short-circuit without checking the predicate. This allows documents from rule-less tables to be returned regardless of the default policy.

The condition should be inverted to ensure the predicate is always evaluated. When tableName is null, the predicate check will apply the default policy:

   async get(arg0: any, arg1?: any): Promise<any> {
     const [tableName, id]: [string | null, GenericId<string>] =
       arg1 !== undefined ? [arg0, arg1] : [this.tableName(arg0), arg0];
     const doc = await this.db.get(id);
     if (doc) {
-      if (tableName && !(await this.predicate(tableName, doc))) {
+      if (tableName === null || !(await this.predicate(tableName!, doc))) {
         return null;
       }
       return doc;
     }
     return null;
   }

The non-null assertion (tableName!) is safe here because the predicate's optional chaining (this.rules[tableName]?.read) handles any string value, including those not in the rules object, applying the default policy accordingly.

🧹 Nitpick comments (1)

packages/convex-helpers/server/rowLevelSecurity.ts (1)

427-428: Type placement is fine but consider adding a brief doc comment.

TypeScript hoists type declarations, so this works correctly. A short JSDoc explaining the purpose of NonUnion would help maintainers:

+/**
+ * Prevents TypeScript from distributing over union types in overload resolution.
+ */
 type NonUnion<T> = T extends never ? never : T;

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a6099d3 and 6896b08.

📒 Files selected for processing (2)

• packages/convex-helpers/server/rowLevelSecurity.test.ts (3 hunks)
• packages/convex-helpers/server/rowLevelSecurity.ts (5 hunks)

🧰 Additional context used

📓 Path-based instructions (1)

**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

When modifying complex TypeScript types, run npm run typecheck in the repository root to ensure types are correct, rather than running tsc manually

Files:

• packages/convex-helpers/server/rowLevelSecurity.test.ts
• packages/convex-helpers/server/rowLevelSecurity.ts

🔇 Additional comments (5)

packages/convex-helpers/server/rowLevelSecurity.test.ts (2)

87-141: LGTM! Well-structured test reorganization.

The describe block cleanly separates implicit and explicit table name API tests. The use of @ts-expect-error is appropriate here since TypeScript doesn't yet know about the 2-argument overload for delete, but you're intentionally testing that new API path.

---

279-391: LGTM! Good coverage for both policy modes.

The tests comprehensively verify default allow and default deny policies for both implicit and explicit table name variants of patch. The @ts-expect-error annotations correctly document that the 3-argument patch API is being exercised.

packages/convex-helpers/server/rowLevelSecurity.ts (3)

335-356: LGTM! checkAuth correctly handles both explicit and inferred table names.

The updated logic properly prioritizes the explicitly provided tableNameArg when available, falling back to inference via this.tableName(id) only when needed.

---

358-375: LGTM! Overload pattern is correct.

The 3-argument vs 2-argument disambiguation via arg2 !== undefined is sound. The @ts-expect-error comment appropriately documents the version dependency on [email protected].

---

396-410: checkAuth is called with explicit table name but the same potential issue exists.

Similar to the get method in WrapReader, if tableName is null when using the single-argument delete form, the checkAuth method will call this.get(id) (line 345), which has the same potential bypass issue noted earlier. The fix to WrapReader.get would address this as well.