Skip to content

Commit 01b4155

Browse files
Programcodeclaude
andcommitted
fix(lint): require one privilege per positive has_*_privilege call
Codex review of 090c4df: `$priv1$SELECT,INSERT$priv1$` and adjacent string concatenation passed. Instead of recognizing each way to spell a list, a positive call's privilege argument must now be one quoted word or a per-row column; anything else is flagged. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
1 parent 090c4df commit 01b4155

2 files changed

Lines changed: 24 additions & 16 deletions

File tree

‎docs/testing.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ Lint runs ESLint, then `npm run lint:repo`:
3333

3434
- ESLint is type-aware for `src/` (outside `src/test/`) and rejects promises nobody awaits or catches. `void` marks a promise that cannot reject or handles its own errors. In browser tests it rejects `waitForTimeout`; a deliberate observation window keeps a `no-restricted-properties` disable comment with its reason.
3535
- [knip](../knip.json) fails on unused files and on dependencies that are unused or missing from `package.json`.
36-
- [`scripts/check-repo-rules.mjs`](../scripts/check-repo-rules.mjs) requires a top-level `permissions:` block, SHA-pinned actions, and `persist-credentials: false` on checkout in every workflow. It rejects a positive `has_*_privilege` check that lists several privileges, since a list means "any of"; write negative checks as `NOT has_*_privilege(...)`, which it recognizes. It runs [squawk](https://squawkhq.com) on migrations newer than its baseline; silence a deliberate lock with `-- squawk-ignore <rule>` and a reason.
36+
- [`scripts/check-repo-rules.mjs`](../scripts/check-repo-rules.mjs) requires a top-level `permissions:` block, SHA-pinned actions, and `persist-credentials: false` on checkout in every workflow. A positive `has_*_privilege` check must name one privilege, as a quoted word or a per-row column, since a list means "any of"; write negative checks as `NOT has_*_privilege(...)`. It runs [squawk](https://squawkhq.com) on migrations newer than its baseline; silence a deliberate lock with `-- squawk-ignore <rule>` and a reason.
3737

3838
Type checking uses `tsconfig.app.json` for the client and `tsconfig.agentic.json` for `e2e.config.ts` and `agentic/` (the e2e runner itself does not type-check), and emits no files. Neither command replaces runtime tests.
3939

‎scripts/check-repo-rules.mjs‎

Lines changed: 23 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -120,42 +120,50 @@ function blankComments(source) {
120120
// whether the call is negated.
121121
const isNegated = (before) => /\bNOT[\s(]*$/i.test(before);
122122

123-
// Arguments of the call whose opening parenthesis is at `open`, up to its
124-
// matching close, skipping parentheses inside quoted and dollar-quoted strings.
123+
// Top-level arguments of the call whose opening parenthesis is at `open`,
124+
// split at commas outside parentheses, quoted and dollar-quoted strings.
125125
function callArguments(source, open) {
126+
const args = [];
126127
let depth = 0;
127128
let quoted = false;
129+
let start = open + 1;
128130
for (let index = open; index < source.length; index += 1) {
129131
const char = source[index];
130-
const dollar = quoted ? null : source.slice(index).match(/^\$[A-Za-z_]*\$/);
132+
const dollar = quoted ? null : source.slice(index).match(/^\$(?:[A-Za-z_][A-Za-z0-9_]*)?\$/);
131133
if (dollar) {
132134
const end = source.indexOf(dollar[0], index + dollar[0].length);
133135
if (end === -1) break;
134136
index = end + dollar[0].length - 1;
135137
} else if (char === "'") quoted = !quoted;
136-
else if (!quoted && char === "(") depth += 1;
137-
else if (!quoted && char === ")" && --depth === 0) return source.slice(open + 1, index);
138+
else if (quoted) continue;
139+
else if (char === "(") depth += 1;
140+
else if (char === "," && depth === 1) {
141+
args.push(source.slice(start, index));
142+
start = index + 1;
143+
} else if (char === ")" && --depth === 0) {
144+
args.push(source.slice(start, index));
145+
return args;
146+
}
138147
}
139-
return source.slice(open + 1);
148+
return args;
140149
}
141150

142151
function checkPgTapPrivileges() {
143152
// has_table_privilege(role, table, 'SELECT,INSERT') is true when ANY listed
144153
// privilege is held, so a list cannot prove that a role has all of them.
145154
// Under NOT, a list correctly means "none of them". The check errs toward
146-
// flagging: a negative written as `= false`, or a list in a comment inside a
147-
// dollar-quoted body, is reported too and is rewritten with NOT.
155+
// flagging: anything but one quoted privilege in a positive call is reported,
156+
// and a negative written as `= false` is rewritten with NOT.
148157
const call = /\bhas_[a-z_]+_privilege\s*\(/gi;
149158
for (const file of listFiles("supabase/tests", [".sql"])) {
150159
const source = blankComments(readFileSync(path.join(root, file), "utf8"));
151160
for (const match of source.matchAll(call)) {
152-
// String arguments, single- or dollar-quoted; the privilege is the last.
153-
const literals = [
154-
...callArguments(source, match.index + match[0].length - 1).matchAll(/'((?:[^']|'')*)'|\$([A-Za-z_]*)\$([\s\S]*?)\$\2\$/g),
155-
];
156-
const last = literals.at(-1);
157-
const privilege = last?.[1] ?? last?.[3] ?? "";
158-
if (privilege.includes(",") && !isNegated(source.slice(0, match.index))) {
161+
// A positive check names one privilege: a plain quoted word, or a column
162+
// that yields one per row (bool_and over unnest). Lists, concatenation,
163+
// dollar quotes and other expressions are flagged.
164+
const privilege = callArguments(source, match.index + match[0].length - 1).at(-1) ?? "";
165+
const single = /^\s*(?:'[A-Za-z ]+'(?:\s*::\s*text)?|[A-Za-z_][A-Za-z0-9_]*)\s*$/.test(privilege);
166+
if (!single && !isNegated(source.slice(0, match.index))) {
159167
report(file, lineOf(source, match.index), "a privilege list means \"any of\": check one privilege per call, or write a negative check as NOT has_*_privilege(...)");
160168
}
161169
}

0 commit comments

Comments
 (0)