Skip to content

Security RLS#481

Open
artemdemo wants to merge 18 commits intomainfrom
security-rls
Open

Security RLS#481
artemdemo wants to merge 18 commits intomainfrom
security-rls

Conversation

@artemdemo
Copy link
Copy Markdown
Contributor

@artemdemo artemdemo commented Apr 15, 2026

Note

Description

This PR implements Row-Level Security (RLS) and Field-Level Security (FLS) for the local dev server's entity API. All CRUD endpoints now decode the JWT from incoming requests to identify the current user, then enforce access control rules defined in the entity schema before reading or writing records. A Windows-compatible fix for function process cleanup is also included.

Related Issue

None

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Refactoring (no functional changes)
  • Other (please describe):

Changes Made

  • Added packages/cli/src/cli/dev/dev-server/db/rls.ts — pure RLS/FLS engine supporting $or, $and, $nor, $in, $nin, $ne, $all operators and {{user.*}} template variables
  • Updated entities-router.ts to decode the Bearer JWT on every request, look up the current user from the users collection, and enforce schema.rls.{read,create,update,delete} and per-field rls.{read,write} on all CRUD endpoints
  • Auto-populates created_by / created_by_id fields from the authenticated user on create
  • Added Database.getSchema() helper to expose entity schemas to the router
  • Fixed FunctionManager.stopAll() to await process exit and use taskkill on Windows for reliable subprocess cleanup
  • Randomised dev-server port in test mode (IS_TEST=true) to prevent parallel-test port collisions
  • Added comprehensive test suites (dev-security*.spec.ts) covering $in, $nin, $nor, $ne, and basic RLS scenarios

Testing

  • I have tested these changes locally
  • I have added/updated tests as needed
  • All tests pass (npm test)

Checklist

  • My code follows the project's style guidelines
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation (if applicable)
  • My changes generate no new warnings
  • I have updated docs/ (AGENTS.md) if I made architectural changes

Additional Notes

RLS denials on read/update/delete return 404 (not 403) to avoid leaking record existence. Create and bulk-create return 403 on denial since no record ID is involved.


🤖 Generated by Claude | 2026-04-20 11:50 UTC | 74f9319

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 15, 2026

🚀 Package Preview Available!


Install this PR's preview build with npm:

npm i @base44-preview/cli@0.0.50-pr.481.74f9319

Prefer not to change any import paths? Install using npm alias so your code still imports base44:

npm i "base44@npm:@base44-preview/cli@0.0.50-pr.481.74f9319"

Or add it to your package.json dependencies:

{
  "dependencies": {
    "base44": "npm:@base44-preview/cli@0.0.50-pr.481.74f9319"
  }
}

Preview published to npm registry — try new features instantly!

Base automatically changed from auth-and-stuff to main April 16, 2026 11:28
Comment on lines +26 to +27
const authData = await readJsonFile(getAuthFilePath());
const result = AuthDataSchema.safeParse(authData);
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Not related to PR.
I just think this is better naming

Comment on lines +42 to +49
outdent`
Deno.serve((req: Request) =>
Response.json({
authorization: req.headers.get("authorization"),
serviceAuthorization: req.headers.get("base44-service-authorization"),
}),
);
`,
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Not related to PR.
Adding outdent for better formatting

@base44 base44 deleted a comment from claude Bot Apr 16, 2026
@base44 base44 deleted a comment from claude Bot Apr 19, 2026
@base44 base44 deleted a comment from claude Bot Apr 20, 2026
@base44 base44 deleted a comment from claude Bot Apr 20, 2026
@base44 base44 deleted a comment from claude Bot Apr 20, 2026
Comment on lines +101 to +113
async stopAll(): Promise<void> {
await Promise.all(
Array.from(this.running, ([name, { process: proc }]) => {
this.logger.log(`Stopping function: ${name}`);
const exited = new Promise<void>((r) => proc.once("exit", () => r()));
if (process.platform === "win32" && proc.pid) {
spawn("taskkill", ["/pid", String(proc.pid), "/T", "/F"]);
} else {
proc.kill();
}
return exited;
}),
);
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Not related to PR, but this fixes flakines (I hope) on windows.
What was happening:
Test in tests/cli/dev.spec.ts was failing - this one: "forwards the service token header from Authorization to local functions"

After investigation with AI we assumed following root cause:

  1. RunLiveHandle.stop() calls child.kill("SIGINT") (CLITestkit.ts:252). Windows doesn't support POSIX signals — Node translates this to abrupt termination, so the CLI's SIGINT→shutdown() handler in main.ts:235 never runs and functionManager.stopAll() is skipped.
  2. The spawned Deno process (function-manager.ts:118) is not in a process group on Windows, so it's orphaned with open handles on .deno/dep_analysis_cache_v2.
  3. tmp-promise's unsafeCleanup then tries to delete <tempDir>/.deno/...EBUSY.

Why .deno lands in the temp dir: testkit sets HOME=tempDir (CLITestkit.ts:76), so Deno defaults DENO_DIR to <tempDir>/.deno.

Solution - to kill process "the windows way"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Very cool investigation, Did this came up from tests / you checked this on windows?

@artemdemo artemdemo marked this pull request as ready for review April 20, 2026 12:04
Comment on lines +204 to +208
const inserted = applyFLS(
stripInternalFields(await collection.insertAsync(record)),
schema,
currentUser,
"read",
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Here and in all cases below - applying FLS check for "read" because at this point I'm returning data to the user.
So I first create - it's one permission, but when I return, I actually read and it's a different permission, so I need to verify again.
I think this only makes sense of FLS (field level security), but there is no reason to do RLS for that object, since user actually created it.
I only need to make sure that user is not getting technical fields that are not related to him.
Not that important for local dev though, but just in case

Copy link
Copy Markdown
Collaborator

@kfirstri kfirstri left a comment

Choose a reason for hiding this comment

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

Went over all files other then rls.ts, need to finish going over it

Comment on lines +101 to +113
async stopAll(): Promise<void> {
await Promise.all(
Array.from(this.running, ([name, { process: proc }]) => {
this.logger.log(`Stopping function: ${name}`);
const exited = new Promise<void>((r) => proc.once("exit", () => r()));
if (process.platform === "win32" && proc.pid) {
spawn("taskkill", ["/pid", String(proc.pid), "/T", "/F"]);
} else {
proc.kill();
}
return exited;
}),
);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Very cool investigation, Did this came up from tests / you checked this on windows?

Comment on lines +58 to +66
let currentUser: UserDocument | undefined;
try {
const auth = req.headers.authorization || "";
const { payload } =
jwt.decode(auth.replace("Bearer ", ""), { complete: true }) ?? {};
currentUser = await db
.getCollection(USER_COLLECTION)
?.findOneAsync<UserDocument>({ email: payload?.sub });
} catch {}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nit: extract user fetching to another function, not really related to "withCollection"

@@ -147,7 +223,7 @@ export async function createEntityRoutes(
router.post(
"/:entityName/bulk",
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Simplification - should maybe the logic for /:entityName and /:entityName/bulk be the same at this point?
It's same logic maintained twice for 1/N items, i guess the 1 item case can use the same bulk function

}

let filteredRecord = db.prepareRecord(entityName, record);
if (schema) {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I dont understand something - why do we check if "schema" exists before applyFLS? schema is always defined AFAIU, maybe you mean if(schema.fls) or something?

?.findOneAsync({ email: payload?.sub });

if (!result) {
if (!currentUser) {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Maybe we can make this withAuth be a shared util and will be used in both routers? currently this logic is implemented inside the withCollection

user: Record<string, unknown>,
): boolean {
for (const [key, expected] of Object.entries(condition)) {
const userValue = key.startsWith("data.") ? user[key.slice(5)] : user[key];
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

isn't this what resolveTemplate does?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants