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
1 change: 1 addition & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,7 @@ source tree — which permanently shadows git's config file. Every subsequent ag
- `Resource` static methods must stay wrapped with `transactional()` — removing this breaks transaction isolation.
- Worker threads (`server/threads/`) receive `workerData.noServerStart = true` to prevent recursive server startup; never start the server inside a worker.
- `contextStorage` (AsyncLocalStorage) carries per-request context (user, transaction) across async boundaries — this is how authorization and transactions work without explicit parameter threading.
- SQL authorization (`verifyPermsAST` → `hasPermissions`) only checks the tables recorded in the statement bucket's affected-attribute map — it iterates that map, so a table missing from it is never checked, and an empty map authorizes by vacuous truth. Two rules follow. Resolve a table reference exactly once, through `sqlEngine/binder/defaultDatabase.ts`, so the authorization layer and the engine's binder cannot disagree about which `database.table` a bare name means. And when adding a new SQL construct, either record its table references in that map or make `getUnauthorizedTableRefs()` report them — an unrecorded reference is a permission bypass, not a missing feature (GHSA-5c29-q62v-jrwf).
- Tests under `unitTests/apiTests/` require the server to be stopped first (`node ./dist/bin/harper.js stop`) — `test:unit:apitests` does this automatically.
- `@export` annotation on a schema class auto-generates a REST API for that table — this is the primary developer-facing API.
- Test style: write new unit tests with `assert` (the bare `node:assert` module) against real modules — **do not add new uses of `sinon` or `rewire`**. Use plain `assert`, **not** `node:assert/strict` — strict mode's deep-equality and coercion rules cause more friction and surprising failures than they prevent; plain `assert` is the house style. When a specific check genuinely needs strict/deep-strict semantics, call `assert.strictEqual`/`assert.deepStrictEqual` explicitly (both exist on plain `assert`) rather than importing `/strict`. This is lint-enforced: oxlint's `no-restricted-imports` rule rejects `node:assert/strict` and `assert/strict` imports. Older tests in `unitTests/security/` and `unitTests/utility/` still depend on them but they are not the target shape; match newer tests in `unitTests/config/*`, `unitTests/resources/*`, `unitTests/components/*`. If you can't write a test without stubbing, comment on the issue describing what's missing and stop — don't reach for sinon/rewire as a shortcut.
12 changes: 9 additions & 3 deletions integrationTests/apiTests/northwind.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -4722,12 +4722,18 @@ suite('Northwind operations', { skip: skipSuite }, (ctx) => {
.expect(200);
});

test('select * FROM orders - test no schema', async () => {
test('select * FROM orders - no schema resolves to the one database that has it', async () => {
// `orders` exists only in `northnwd`, so authorization and the engine both resolve the
// bare name to it and the query runs like the schema-qualified equivalent
// (GHSA-5c29-q62v-jrwf: resolution now happens once, before authorization).
await client
.req()
.send({ operation: 'sql', sql: 'select * FROM orders' })
.expect((r) => assert.equal(r.body.error, 'schema not defined for table orders', r.text))
.expect(500);
.expect((r) => {
assert.equal(r.body.length, 830, r.text);
assert.equal(r.body[0].orderid, 10248, r.text);
})
.expect(200);
});

test('select * from call.aggr - reserved words', async () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
# Regression fixture for GHSA-5c29-q62v-jrwf — schema-unqualified SQL must be authorized.
graphqlSchema:
files: '*.graphql'
rest: true
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# Regression fixture for GHSA-5c29-q62v-jrwf — schema-unqualified SQL must be authorized.
#
# `UnqualSecret` is the forbidden table: the test role holds no grant on it at all.
# `UnqualPublic` is the permitted table: the role can read it, so the test can prove the
# fix still resolves a legitimate unqualified reference instead of rejecting everything.
# Both names are unique across databases so the engine's pickDefaultDatabase resolves them.
type UnqualSecret @table @export {
id: ID @primaryKey
owner: String
ssn: String
}

type UnqualPublic @table @export {
id: ID @primaryKey
label: String
}

# Lives in a SECOND database so a bare, unaliased cross-database join can be exercised. The role
# below gets table-level read on it but is denied the `ssn` attribute — the column's database has
# to resolve through the table-to-schema map or its attribute permission is never checked.
type UnqualVault @table(database: "vault") @export {
id: ID @primaryKey
label: String
ssn: String
}
Loading
Loading