fix(cli): declare @wrnexus/authz dependency, exit cleanly on bad authz input
Round-1 review fixes for Task 13: - packages/cli/package.json was missing @wrnexus/authz, and packages/authz/package.json was missing @wrnexus/core despite importing its types in index.ts/middleware.ts/advanced.ts. Both only worked in-repo because bare "@wrnexus/*" specifiers resolve through the root tsconfig.json paths map; a standalone install of @wrnexus/cli or @wrnexus/authz would fail at runtime. - authz.ts's unknown/missing-subcommand and bad --dialect paths now console.error + process.exit(1), matching db.ts's convention, instead of throwing — index.ts's top-level catch previously printed those as a raw stack trace. Added a subprocess-level test that spawns the real CLI and asserts stderr has the usage line with no stack frame. - nextMigrationNumber now extracts the leading-digit run the same way db/migrate.ts's nextNumber does, instead of a fixed slice(0, 4) that would have undercounted once a migration number passed 9999.
This commit is contained in:
@@ -9,6 +9,7 @@
|
|||||||
"./db": "./src/db.ts"
|
"./db": "./src/db.ts"
|
||||||
},
|
},
|
||||||
"dependencies": {
|
"dependencies": {
|
||||||
|
"@wrnexus/core": "workspace:*",
|
||||||
"@wrnexus/db": "workspace:*"
|
"@wrnexus/db": "workspace:*"
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -23,6 +23,7 @@
|
|||||||
"@wrnexus/mcp": "workspace:*",
|
"@wrnexus/mcp": "workspace:*",
|
||||||
"@wrnexus/playground": "workspace:*",
|
"@wrnexus/playground": "workspace:*",
|
||||||
"@wrnexus/db": "workspace:*",
|
"@wrnexus/db": "workspace:*",
|
||||||
|
"@wrnexus/authz": "workspace:*",
|
||||||
"@wrnexus/plugin": "workspace:*",
|
"@wrnexus/plugin": "workspace:*",
|
||||||
"@wrnexus/syntax": "workspace:*",
|
"@wrnexus/syntax": "workspace:*",
|
||||||
"@wrnexus/typecheck": "workspace:*",
|
"@wrnexus/typecheck": "workspace:*",
|
||||||
|
|||||||
+20
-10
@@ -41,23 +41,33 @@ export async function loadAuthzCatalog(appDir: string): Promise<AuthzCatalog> {
|
|||||||
return mergeCatalogs(sources);
|
return mergeCatalogs(sources);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/** Print a message and exit non-zero, matching db.ts's convention for user-facing
|
||||||
|
* CLI errors: never throw, so index.ts's generic `main().catch` handler (which
|
||||||
|
* prints the raw error, stack and all) is never reached for an expected failure. */
|
||||||
|
function fail(message: string): never {
|
||||||
|
console.error(message);
|
||||||
|
process.exit(1);
|
||||||
|
}
|
||||||
|
|
||||||
|
// Same leading-digit extraction as db/migrate.ts's `nextNumber`: a fixed
|
||||||
|
// `slice(0, 4)` would undercount once a migration number grows past 9999.
|
||||||
function nextMigrationNumber(dir: string): string {
|
function nextMigrationNumber(dir: string): string {
|
||||||
if (!existsSync(dir)) return "0001";
|
if (!existsSync(dir)) return "0001";
|
||||||
const numbers = readdirSync(dir)
|
let max = 0;
|
||||||
.map((name) => Number.parseInt(name.slice(0, 4), 10))
|
for (const name of readdirSync(dir)) {
|
||||||
.filter((value) => Number.isInteger(value));
|
const match = /^(\d+)/.exec(name);
|
||||||
return String((numbers.length ? Math.max(...numbers) : 0) + 1).padStart(4, "0");
|
if (match) max = Math.max(max, Number(match[1]));
|
||||||
|
}
|
||||||
|
return String(max + 1).padStart(4, "0");
|
||||||
}
|
}
|
||||||
|
|
||||||
/** Parse `--dialect=<value>` from CLI args. Defaults to sqlite; rejects unknown values. */
|
/** Parse `--dialect=<value>` from CLI args. Defaults to sqlite; rejects unknown values. */
|
||||||
function resolveDialect(args: string[]): Dialect {
|
function resolveDialect(args: string[]): Dialect {
|
||||||
const flag = args.find((arg) => arg.startsWith("--dialect="));
|
const flag = args.find((arg) => arg.startsWith("--dialect="));
|
||||||
if (!flag) return "sqlite";
|
if (!flag) return "sqlite";
|
||||||
const value = flag.split("=")[1];
|
const value = flag.split("=")[1] ?? "";
|
||||||
if ((DIALECTS as readonly string[]).includes(value ?? "")) return value as Dialect;
|
if ((DIALECTS as readonly string[]).includes(value)) return value as Dialect;
|
||||||
throw new Error(
|
return fail(`Unrecognised --dialect='${value}'. Use one of: ${DIALECTS.join(", ")}.`);
|
||||||
`WRN-AUTHZ-INIT: unrecognised --dialect='${value}'. Use one of: ${DIALECTS.join(", ")}.`,
|
|
||||||
);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
export async function runAuthzCommand(
|
export async function runAuthzCommand(
|
||||||
@@ -118,6 +128,6 @@ export async function runAuthzCommand(
|
|||||||
}
|
}
|
||||||
|
|
||||||
default:
|
default:
|
||||||
throw new Error(USAGE);
|
fail(USAGE);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1,4 +1,4 @@
|
|||||||
import { afterAll, describe, expect, test } from "bun:test";
|
import { afterAll, describe, expect, spyOn, test } from "bun:test";
|
||||||
import {
|
import {
|
||||||
mkdirSync,
|
mkdirSync,
|
||||||
mkdtempSync,
|
mkdtempSync,
|
||||||
@@ -45,6 +45,31 @@ afterAll(() => {
|
|||||||
rmSync(scratchRoot, { recursive: true, force: true });
|
rmSync(scratchRoot, { recursive: true, force: true });
|
||||||
});
|
});
|
||||||
|
|
||||||
|
/**
|
||||||
|
* authz.ts's `fail()` helper (usage errors, bad --dialect) mirrors db.ts's
|
||||||
|
* convention: console.error + process.exit(1), never throw — so index.ts's
|
||||||
|
* generic `main().catch(err) { console.error(err); process.exit(1); }` (which
|
||||||
|
* prints the raw Error, stack and all) never sees an expected validation
|
||||||
|
* failure. That means a *direct* call to runAuthzCommand() would normally kill
|
||||||
|
* the whole test worker via a real process.exit(); intercept both console.error
|
||||||
|
* and process.exit so the failure path stays testable in-process.
|
||||||
|
*/
|
||||||
|
async function expectCleanFailure(run: () => Promise<void>): Promise<string> {
|
||||||
|
const errors: string[] = [];
|
||||||
|
const originalError = console.error;
|
||||||
|
console.error = (...args: unknown[]) => void errors.push(args.join(" "));
|
||||||
|
const exitSpy = spyOn(process, "exit").mockImplementation(((code?: number) => {
|
||||||
|
throw new Error(`__process_exit_${code}__`);
|
||||||
|
}) as never);
|
||||||
|
try {
|
||||||
|
await expect(run()).rejects.toThrow(/^__process_exit_1__$/);
|
||||||
|
} finally {
|
||||||
|
console.error = originalError;
|
||||||
|
exitSpy.mockRestore();
|
||||||
|
}
|
||||||
|
return errors.join("\n");
|
||||||
|
}
|
||||||
|
|
||||||
describe("wrnexus authz", () => {
|
describe("wrnexus authz", () => {
|
||||||
test("loadAuthzCatalog merges every declaration", async () => {
|
test("loadAuthzCatalog merges every declaration", async () => {
|
||||||
const catalog = await loadAuthzCatalog(join(scaffold(), "app"));
|
const catalog = await loadAuthzCatalog(join(scaffold(), "app"));
|
||||||
@@ -87,14 +112,38 @@ describe("wrnexus authz", () => {
|
|||||||
expect(output).toContain("editor");
|
expect(output).toContain("editor");
|
||||||
});
|
});
|
||||||
|
|
||||||
test("an unknown subcommand throws with usage", async () => {
|
test("an unknown subcommand prints usage and exits 1, not a thrown error", async () => {
|
||||||
await expect(runAuthzCommand(scaffold(), "bogus", [])).rejects.toThrow(/usage/i);
|
const errorOutput = await expectCleanFailure(() => runAuthzCommand(scaffold(), "bogus", []));
|
||||||
|
expect(errorOutput).toMatch(/usage/i);
|
||||||
});
|
});
|
||||||
|
|
||||||
test("a missing subcommand throws with usage", async () => {
|
test("a missing subcommand prints usage and exits 1", async () => {
|
||||||
await expect(runAuthzCommand(scaffold(), undefined, [])).rejects.toThrow(/usage/i);
|
const errorOutput = await expectCleanFailure(() => runAuthzCommand(scaffold(), undefined, []));
|
||||||
|
expect(errorOutput).toMatch(/usage/i);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test("CLI subprocess: unknown subcommand prints usage without a stack trace", async () => {
|
||||||
|
const cliEntry = join(import.meta.dir, "..", "src", "index.ts");
|
||||||
|
const root = scaffold();
|
||||||
|
const proc = Bun.spawn({
|
||||||
|
cmd: ["bun", cliEntry, "authz", "bogus"],
|
||||||
|
cwd: root,
|
||||||
|
env: { ...process.env, WRNEXUS_NO_UPDATE_CHECK: "1" },
|
||||||
|
stdout: "pipe",
|
||||||
|
stderr: "pipe",
|
||||||
|
});
|
||||||
|
const [stderr] = await Promise.all([
|
||||||
|
new Response(proc.stderr).text(),
|
||||||
|
new Response(proc.stdout).text(),
|
||||||
|
]);
|
||||||
|
const exitCode = await proc.exited;
|
||||||
|
expect(exitCode).not.toBe(0);
|
||||||
|
expect(stderr).toMatch(/usage/i);
|
||||||
|
// A raw Error/stack trace looks like "at <fn> (file.ts:12:34)"; the clean
|
||||||
|
// console.error(usage) + process.exit(1) path never produces that shape.
|
||||||
|
expect(stderr).not.toMatch(/at .*\.ts:\d+/);
|
||||||
|
}, 15000);
|
||||||
|
|
||||||
test("list does not crash on an app with no app/authz directory", async () => {
|
test("list does not crash on an app with no app/authz directory", async () => {
|
||||||
mkdirSync(scratchRoot, { recursive: true });
|
mkdirSync(scratchRoot, { recursive: true });
|
||||||
const root = mkdtempSync(join(scratchRoot, "empty-"));
|
const root = mkdtempSync(join(scratchRoot, "empty-"));
|
||||||
@@ -160,7 +209,10 @@ describe("wrnexus authz", () => {
|
|||||||
test("init with an unrecognised --dialect= does not silently fall back to sqlite", async () => {
|
test("init with an unrecognised --dialect= does not silently fall back to sqlite", async () => {
|
||||||
const root = scaffold();
|
const root = scaffold();
|
||||||
mkdirSync(join(root, "app", "db", "migrations"), { recursive: true });
|
mkdirSync(join(root, "app", "db", "migrations"), { recursive: true });
|
||||||
await expect(runAuthzCommand(root, "init", ["--dialect=oracle"])).rejects.toThrow(/dialect/i);
|
const errorOutput = await expectCleanFailure(() =>
|
||||||
|
runAuthzCommand(root, "init", ["--dialect=oracle"]),
|
||||||
|
);
|
||||||
|
expect(errorOutput).toMatch(/dialect/i);
|
||||||
});
|
});
|
||||||
|
|
||||||
test("init writes a migration that the migration runner can parse", async () => {
|
test("init writes a migration that the migration runner can parse", async () => {
|
||||||
@@ -178,6 +230,21 @@ describe("wrnexus authz", () => {
|
|||||||
expect(migration.down).toContain("DROP TABLE IF EXISTS _wrn_authz_assignment");
|
expect(migration.down).toContain("DROP TABLE IF EXISTS _wrn_authz_assignment");
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test("init numbers the next migration correctly past a 5-digit prefix", async () => {
|
||||||
|
// nextMigrationNumber originally sliced the first 4 characters of the
|
||||||
|
// filename, which would have parsed "10000_big.sql" as "1000" and reused
|
||||||
|
// that number instead of advancing past it. It must match db/migrate.ts's
|
||||||
|
// leading-digit regex instead.
|
||||||
|
const root = scaffold();
|
||||||
|
const dir = join(root, "app", "db", "migrations");
|
||||||
|
mkdirSync(dir, { recursive: true });
|
||||||
|
writeFileSync(join(dir, "0001_users.sql"), "-- +up\n\n-- +down\n", "utf8");
|
||||||
|
writeFileSync(join(dir, "10000_big.sql"), "-- +up\n\n-- +down\n", "utf8");
|
||||||
|
await runAuthzCommand(root, "init", []);
|
||||||
|
const file = readdirSync(dir).find((name) => name.includes("authz"));
|
||||||
|
expect(file).toBe("10001_authz_tables.sql");
|
||||||
|
});
|
||||||
|
|
||||||
test("generate does not clobber a real declaration file", async () => {
|
test("generate does not clobber a real declaration file", async () => {
|
||||||
const root = scaffold();
|
const root = scaffold();
|
||||||
await runAuthzCommand(root, "generate", []);
|
await runAuthzCommand(root, "generate", []);
|
||||||
|
|||||||
Reference in New Issue
Block a user