From f4e93f339dda45b3693a5c8c8b1ae695e7b5a1cc Mon Sep 17 00:00:00 2001 From: diegosouzapw Date: Tue, 4 Aug 2026 18:32:28 -0300 Subject: [PATCH] docs: add management authentication terminology guide (#7786) --- .../7786-management-auth-terminology-docs.md | 1 + .../feat-7786/docs/guides/MANAGEMENT-AUTH.md | 41 ++++++ .../tests/unit/management-auth-docs.test.ts | 27 ++++ changelog.d/fixes/9159-fix.plan.md | 1 + tests/unit/authz/probe-9033-repro.test.ts | 128 ++++++++++++++++++ tests/unit/repro-8522.test.ts | 45 ++++++ tests/unit/repro-8956.test.ts | 65 +++++++++ 7 files changed, 308 insertions(+) create mode 100644 .claude/worktrees/feat-7786/changelog.d/features/7786-management-auth-terminology-docs.md create mode 100644 .claude/worktrees/feat-7786/docs/guides/MANAGEMENT-AUTH.md create mode 100644 .claude/worktrees/feat-7786/tests/unit/management-auth-docs.test.ts create mode 100644 changelog.d/fixes/9159-fix.plan.md create mode 100644 tests/unit/authz/probe-9033-repro.test.ts create mode 100644 tests/unit/repro-8522.test.ts create mode 100644 tests/unit/repro-8956.test.ts diff --git a/.claude/worktrees/feat-7786/changelog.d/features/7786-management-auth-terminology-docs.md b/.claude/worktrees/feat-7786/changelog.d/features/7786-management-auth-terminology-docs.md new file mode 100644 index 0000000000..4a5fca7f9d --- /dev/null +++ b/.claude/worktrees/feat-7786/changelog.d/features/7786-management-auth-terminology-docs.md @@ -0,0 +1 @@ +- **docs:** add management authentication terminology guide ([#7786](https://github.com/diegosouzapw/OmniRoute/issues/7786)) diff --git a/.claude/worktrees/feat-7786/docs/guides/MANAGEMENT-AUTH.md b/.claude/worktrees/feat-7786/docs/guides/MANAGEMENT-AUTH.md new file mode 100644 index 0000000000..cc74622a5f --- /dev/null +++ b/.claude/worktrees/feat-7786/docs/guides/MANAGEMENT-AUTH.md @@ -0,0 +1,41 @@ +# Management Authentication + +OmniRoute uses four distinct credential families for management access. This guide +distinguishes them by purpose, scope, and locality. + +| Credential | Scope | Locality | Use Case | +|-------------------------|--------------------|---------------|-----------------------------------| +| Dashboard JWT session | Full management | Localhost | Web dashboard login | +| CLI machine-id token | Full management | Per-machine | `omniroute` CLI commands | +| Scoped `oma_` token | Configurable scope | External | Automation / CI / API access | +| Manage-scope API key | `manage` scope | External | Management API calls | + +## Dashboard JWT Session + +Generated on dashboard login (`/api/auth/login`). Stored in HTTP-only cookie. +Valid for the session duration. Cannot be used from external hosts. + +## CLI Machine-ID Token + +Created by `omniroute auth login` on first use. Stored in `~/.omniroute/auth.json`. +Used by the CLI for all management operations. Tied to the machine identity. + +## Scoped `oma_` Access Token + +Created via dashboard or CLI with configurable scopes (e.g., `manage`, `read`). +Format: `oma_`. Used for programmatic access from external systems. + +## Manage-Scope API Key + +Standard API key with the `manage` scope enabled. Created in dashboard API Keys page. +Used for management API calls from external hosts. + +## Header Examples + +``` +Authorization: Bearer oma_abc123def456 +Authorization: Bearer +Cookie: omniroute_session= +``` + +See `docs/reference/API_REFERENCE.md` for endpoint-specific auth requirements. diff --git a/.claude/worktrees/feat-7786/tests/unit/management-auth-docs.test.ts b/.claude/worktrees/feat-7786/tests/unit/management-auth-docs.test.ts new file mode 100644 index 0000000000..35410e81c3 --- /dev/null +++ b/.claude/worktrees/feat-7786/tests/unit/management-auth-docs.test.ts @@ -0,0 +1,27 @@ +import { describe, it } from "node:test"; +import { ok } from "node:assert/strict"; +import { readFileSync } from "node:fs"; + +describe("Management auth documentation (#7786)", () => { + const docPath = "docs/guides/MANAGEMENT-AUTH.md"; + const content = readFileSync(docPath, "utf-8"); + + it("exists and has content", () => { + ok(content.length > 500, "should have substantial content"); + ok(content.includes("Dashboard JWT session")); + ok(content.includes("CLI machine-id token")); + ok(content.includes("oma_")); + }); + + it("documents all four credential families", () => { + const families = ["Dashboard JWT", "CLI machine-id", "oma_", "Manage-scope"]; + for (const f of families) { + ok(content.includes(f), `should document ${f}`); + } + }); + + it("mentions relevant auth header examples", () => { + ok(content.includes("Authorization")); + ok(content.includes("Bearer")); + }); +}); diff --git a/changelog.d/fixes/9159-fix.plan.md b/changelog.d/fixes/9159-fix.plan.md new file mode 100644 index 0000000000..22d84fba2a --- /dev/null +++ b/changelog.d/fixes/9159-fix.plan.md @@ -0,0 +1 @@ +- fix(management): authorize mcp:connect-only keys on loopback/LAN when requireLogin is enabled (#9159) \ No newline at end of file diff --git a/tests/unit/authz/probe-9033-repro.test.ts b/tests/unit/authz/probe-9033-repro.test.ts new file mode 100644 index 0000000000..563739d34f --- /dev/null +++ b/tests/unit/authz/probe-9033-repro.test.ts @@ -0,0 +1,128 @@ +// Repro test for #9033 — IP blacklist does not block on direct connections +// and does not propagate without restart. +// D1: blacklisted IP on a DIRECT connection (trusted peer stamp, no XFF) is NOT blocked +// D2: persisted config written after first load is never re-read by the loaded instance +// Bonus: ipFilterModeSchema rejects "whitelist-priority" that the UI offers and checkIP implements +import test from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { NextRequest } from "next/server"; + +const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-9033-repro-")); +process.env.DATA_DIR = TEST_DATA_DIR; +process.env.JWT_SECRET = "test-secret-9033"; + +const core = await import("../../../src/lib/db/core.ts"); +const ipFilter = await import("../../../open-sse/services/ipFilter.ts"); +const pipeline = await import("../../../src/server/authz/pipeline.ts"); + +const ORIGINAL_STAMP_TOKEN = process.env.OMNIROUTE_PEER_STAMP_TOKEN; + +test.after(() => { + core.resetDbInstance(); + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true }); + if (ORIGINAL_STAMP_TOKEN === undefined) delete process.env.OMNIROUTE_PEER_STAMP_TOKEN; + else process.env.OMNIROUTE_PEER_STAMP_TOKEN = ORIGINAL_STAMP_TOKEN; +}); + +test.beforeEach(() => { + core.resetDbInstance(); + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true }); + fs.mkdirSync(TEST_DATA_DIR, { recursive: true }); + ipFilter.resetIPFilter(); + delete process.env.OMNIROUTE_PEER_STAMP_TOKEN; +}); + +const BLOCKED = "203.0.113.99"; + +function makeRequest(extraHeaders: Record = {}) { + return new NextRequest("http://localhost/v1/models", { + headers: { ...extraHeaders }, + }); +} + +test("D1: blacklisted IP is blocked on a DIRECT connection (trusted peer stamp, no XFF)", async () => { + process.env.OMNIROUTE_PEER_STAMP_TOKEN = "stamp-tok"; + ipFilter.configureIPFilter({ enabled: true, mode: "blacklist" }); + ipFilter.addToBlacklist(BLOCKED); + + // Simulate a direct connection: the peer stamp says the client is BLOCKED, + // and there is no x-forwarded-for header (direct connection, not via proxy). + const res = await pipeline.runAuthzPipeline( + makeRequest({ "x-omniroute-peer-ip": "stamp-tok|203.0.113.99" }), + { enforce: true } + ); + + assert.equal( + res.status, + 403, + `direct blacklisted IP must be blocked, got status=${res.status}` + ); +}); + +test("D2: persisted config written after first load is honored WITHOUT restart", async () => { + // Simulate: the settings route (separate module instance) writes config to DB. + // The ipFilter module instance (already loaded) must re-read it. + // First, load the module once (simulates initial load from a previous request). + ipFilter.resetIPFilter(); + ipFilter.configureIPFilter({ enabled: true, mode: "blacklist" }); + ipFilter.addToBlacklist(BLOCKED); + assert.equal(ipFilter.checkIP(BLOCKED).allowed, false, "blacklist must be active after config"); + + // Now simulate a "settings route" write: write directly to the DB key_value table + // with a DIFFERENT config (e.g. empty blacklist, effectively "allow all"). + const db = core.getDbInstance(); + db.prepare( + "INSERT OR REPLACE INTO key_value (namespace, key, value) VALUES (?, ?, ?)" + ).run( + "ipFilter", + "config", + JSON.stringify({ enabled: true, mode: "blacklist", blacklist: [], whitelist: [] }) + ); + + // Without a restart, the ipFilter instance must re-read from DB on next checkIP call. + // The BLOCKED IP should NOT be blocked anymore because the DB config has empty blacklist. + const result = ipFilter.checkIP(BLOCKED); + assert.equal( + result.allowed, + true, + `stale-config enforcer must re-read DB, got: ${JSON.stringify(result)}` + ); +}); + +test("D3: behind reverse proxy (peer stamp=loopback + via-proxy marker + XFF=blacklisted IP) still blocks", async () => { + process.env.OMNIROUTE_PEER_STAMP_TOKEN = "stamp-tok"; + ipFilter.configureIPFilter({ enabled: true, mode: "blacklist" }); + ipFilter.addToBlacklist(BLOCKED); + + // Behind a reverse proxy: the peer IP is the proxy hop (127.0.0.1), + // the via-proxy marker is set, and the real client IP is in x-forwarded-for. + const res = await pipeline.runAuthzPipeline( + makeRequest({ + "x-omniroute-peer-ip": "stamp-tok|127.0.0.1", + "x-omniroute-via-proxy": "stamp-tok|1", + "x-forwarded-for": BLOCKED, + }), + { enforce: true } + ); + + assert.equal( + res.status, + 403, + `behind-proxy blacklisted IP must be blocked, got status=${res.status}` + ); +}); + +test("Bonus: ipFilterModeSchema accepts whitelist-priority", async () => { + const { ipFilterModeSchema } = await import( + "../../../src/shared/validation/schemas/misc.ts" + ); + const result = ipFilterModeSchema.safeParse("whitelist-priority"); + assert.equal( + result.success, + true, + `ipFilterModeSchema must accept "whitelist-priority", got: ${JSON.stringify(result)}` + ); +}); \ No newline at end of file diff --git a/tests/unit/repro-8522.test.ts b/tests/unit/repro-8522.test.ts new file mode 100644 index 0000000000..c6d4dd6c30 --- /dev/null +++ b/tests/unit/repro-8522.test.ts @@ -0,0 +1,45 @@ +/** + * repro-8522 — quality-gate inherited-drift defect. + * + * Issue #8522: check:file-size (and the eslint-suppressions count) are ABSOLUTE + * ratchets with no base-ref comparison. Once the release base is over a frozen + * cap (inherited drift from an already-merged PR), EVERY subsequent PR goes red + * on that gate regardless of content — the "innocent PR" cannot pass, so red + * stops distinguishing "you broke it" from "you exist". + * + * This test reproduces the minimal defect: an innocent PR (base and head have + * IDENTICAL LOC on the frozen file, PR touched nothing) still produces a + * violation, because `evaluateFileSizes` compares head LOC to the frozen number + * with no notion of the base. + */ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { evaluateFileSizes } from "../../scripts/check/check-file-size.mjs"; + +test("8522: innocent PR (base already over frozen cap) must NOT be a violation", () => { + // Scenario: frozen cap for src/foo.ts is 100. Some earlier merged PR grew it + // to 110. The base of THIS PR is therefore 110. This PR is innocent — it does + // not touch src/foo.ts at all, so head LOC == base LOC == 110. + const baseLocByFile = { "src/foo.ts": 110 }; + const currentLocByFile = { ...baseLocByFile }; // PR changed nothing in foo.ts + const frozen = { "src/foo.ts": 100 }; + const cap = 100; + + const { violations } = evaluateFileSizes(currentLocByFile, frozen, cap); + + // The gate has no base-ref input; it compares head LOC (110) to frozen (100) + // and flags a violation. But the PR introduced ZERO growth — it is a false + // positive on inherited drift. + assert.deepEqual(violations, [], "innocent PR flagged for inherited drift"); +}); + +test("8522: PR that DOES grow a frozen file above frozen cap is a violation", () => { + // Sanity: the gate must still catch a PR that grows the file above its cap. + const baseLocByFile = { "src/foo.ts": 100 }; + const currentLocByFile = { "src/foo.ts": 112 }; // PR grew it +12 + const frozen = { "src/foo.ts": 100 }; + const cap = 100; + + const { violations } = evaluateFileSizes(currentLocByFile, frozen, cap); + assert.equal(violations.length, 1, "own-growth PR must be a violation"); +}); diff --git a/tests/unit/repro-8956.test.ts b/tests/unit/repro-8956.test.ts new file mode 100644 index 0000000000..daf76a4332 --- /dev/null +++ b/tests/unit/repro-8956.test.ts @@ -0,0 +1,65 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; + +const { resolveProjectRoot } = await import("../../src/lib/system/autoUpdate.ts"); + +test("repro-8956: resolveProjectRoot skips synthetic .build/next/package.json (no name field)", () => { + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "repro-8956-")); + try { + // Simulate a Next.js standalone build layout inside a real repo: + // /repo/.git/ (real git marker) + // /repo/package.json (real repo root, has a "name" field) + // /repo/.build/next/package.json (synthetic marker, {"type":"commonjs"}, no name) + // /repo/.build/next/server/chunks/ (where the bundled module lives at runtime) + const repoRoot = path.join(tmp, "repo"); + const buildPkgDir = path.join(repoRoot, ".build", "next"); + const chunksDir = path.join(buildPkgDir, "server", "chunks"); + + fs.mkdirSync(path.join(repoRoot, ".git"), { recursive: true }); + fs.mkdirSync(chunksDir, { recursive: true }); + + // Real root package.json with a name + fs.writeFileSync(path.join(repoRoot, "package.json"), JSON.stringify({ name: "omniroute" })); + // Synthetic Next.js standalone build marker — no "name" field + fs.writeFileSync(path.join(buildPkgDir, "package.json"), JSON.stringify({ type: "commonjs" })); + + // Start from the chunks dir (simulating __dirname at runtime) + const root = resolveProjectRoot("/fallback", chunksDir); + + // Must NOT stop at .build/next — must walk up to the repo root that has .git + assert.equal( + root, + repoRoot, + `resolveProjectRoot returned ${root}, expected the repo root ${repoRoot} ` + + "(it stopped at the synthetic .build/next/package.json marker)" + ); + + // The resolved root must own .git so source-mode validation passes + assert.ok( + fs.existsSync(path.join(root, ".git")), + `PROJECT_ROOT resolved to ${root}, which lacks .git` + ); + } finally { + fs.rmSync(tmp, { recursive: true, force: true }); + } +}); + +test("repro-8956: resolveProjectRoot still finds package.json with a name field", () => { + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "repro-8956-named-")); + try { + // A normal repo root: has .git AND a named package.json + const repoRoot = path.join(tmp, "my-repo"); + const subDir = path.join(repoRoot, "some", "deep", "path"); + fs.mkdirSync(subDir, { recursive: true }); + fs.writeFileSync(path.join(repoRoot, "package.json"), JSON.stringify({ name: "my-app" })); + fs.mkdirSync(path.join(repoRoot, ".git"), { recursive: true }); + + const root = resolveProjectRoot("/fallback", subDir); + assert.equal(root, repoRoot); + } finally { + fs.rmSync(tmp, { recursive: true, force: true }); + } +});