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 .env.example
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ MACHINE_ID_SALT=endpoint-proxy-salt
ENABLE_REQUEST_LOGS=false
AUTH_COOKIE_SECURE=false
REQUIRE_API_KEY=false
ALLOW_API_KEY_REVEAL=false

# Input Sanitizer (FASE-01 — prompt injection & PII protection)
# INPUT_SANITIZER_ENABLED=true
Expand Down
1 change: 1 addition & 0 deletions docs/USER_GUIDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -519,6 +519,7 @@ post_install() {
| `CLOUD_URL` | `https://omniroute.dev` | Cloud sync endpoint base URL |
| `API_KEY_SECRET` | `endpoint-proxy-api-key-secret` | HMAC secret for generated API keys |
| `REQUIRE_API_KEY` | `false` | Enforce Bearer API key on `/v1/*` |
| `ALLOW_API_KEY_REVEAL` | `false` | Allow Api Manager to copy full API keys on demand |
| `ENABLE_REQUEST_LOGS` | `false` | Enables request/response logs |
| `AUTH_COOKIE_SECURE` | `false` | Force `Secure` auth cookie (behind HTTPS reverse proxy) |
| `OMNIROUTE_MEMORY_MB` | `512` | Node.js heap limit in MB |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ export default function ApiManagerPageClient() {
const [isSubmitting, setIsSubmitting] = useState(false);
const [usageStats, setUsageStats] = useState<Record<string, KeyUsageStats>>({});
const [sessionCounts, setSessionCounts] = useState<Record<string, number>>({});
const [allowKeyReveal, setAllowKeyReveal] = useState(false);

const { copied, copy } = useCopyToClipboard();

Expand Down Expand Up @@ -150,6 +151,7 @@ export default function ApiManagerPageClient() {
if (res.ok) {
const data = await res.json();
setKeys(data.keys || []);
setAllowKeyReveal(data.allowKeyReveal === true);
// Fetch usage stats after keys are loaded
fetchUsageStats(data.keys || []);
fetchSessionCounts(data.keys || []);
Expand Down Expand Up @@ -288,6 +290,25 @@ export default function ApiManagerPageClient() {
setShowPermissionsModal(true);
};

const handleCopyExistingKey = async (keyId: string) => {
if (!keyId) return;

try {
const res = await fetch(`/api/keys/${encodeURIComponent(keyId)}/reveal`);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

WARNING: No user feedback on failure — When the reveal request fails (e.g., network error, server error), the user is not informed. The error is only logged to console.

Consider adding error state to inform users why the copy operation failed.

if (!res.ok) {
console.log("Error revealing key:", await res.text());
return;
}

const data = await res.json();
if (typeof data?.key === "string") {
await copy(data.key, `existing_key_${keyId}`);
}
} catch (error) {
console.log("Error copying existing key:", error);
}
};
Comment on lines +293 to +310

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The current implementation of handleCopyExistingKey logs errors to the console, which is not visible to the user. For a better user experience, it's recommended to use the existing setError state to display a user-facing error message in the UI, similar to how other actions in this component are handled. This provides clear feedback to the user if the copy action fails. You may need to add new translation keys like failedToRevealKey and failedToCopyKey.

  const handleCopyExistingKey = async (keyId: string) => {
    if (!keyId) return;

    clearError();
    try {
      const res = await fetch(`/api/keys/${encodeURIComponent(keyId)}/reveal`);
      if (!res.ok) {
        const data = await res.json().catch(() => ({ error: t("failedToRevealKey") }));
        setError(data.error || t("failedToRevealKey"));
        return;
      }

      const data = await res.json();
      if (typeof data?.key === "string") {
        await copy(data.key, `existing_key_${keyId}`);
      }
    } catch (error) {
      console.error("Error copying existing key:", error);
      setError(t("failedToCopyKey"));
    }
  };


const handleUpdatePermissions = async (
allowedModels: string[],
noLog: boolean,
Expand Down Expand Up @@ -560,12 +581,25 @@ export default function ApiManagerPageClient() {
</div>
<div className="col-span-3 flex items-center gap-1.5">
<code className="text-sm text-text-muted font-mono truncate">{key.key}</code>
<span
className="p-1 text-text-muted/40 opacity-0 group-hover:opacity-100 transition-all shrink-0 cursor-help"
title={t("keyOnlyAvailableAtCreation")}
>
<span className="material-symbols-outlined text-[14px]">lock</span>
</span>
{allowKeyReveal ? (
<button
onClick={() => handleCopyExistingKey(key.id)}
className="p-1 text-text-muted/60 hover:text-primary transition-colors shrink-0"
title={tc("copy")}
aria-label={tc("copy")}
>
<span className="material-symbols-outlined text-[14px]">
{copied === `existing_key_${key.id}` ? "check" : "content_copy"}
</span>
</button>
) : (
<span
className="p-1 text-text-muted/40 opacity-0 group-hover:opacity-100 transition-all shrink-0 cursor-help"
title={t("keyOnlyAvailableAtCreation")}
>
<span className="material-symbols-outlined text-[14px]">lock</span>
</span>
)}
</div>
<div className="col-span-2 flex items-center">
<div className="flex flex-col items-start gap-1">
Expand Down
24 changes: 24 additions & 0 deletions src/app/api/keys/[id]/reveal/route.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import { NextResponse } from "next/server";
import { getApiKeyById } from "@/lib/localDb";
import { isApiKeyRevealEnabled } from "@/lib/apiKeyExposure";

// GET /api/keys/[id]/reveal - Reveal full API key for explicit copy actions
export async function GET(_request, { params }) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

CRITICAL: Missing authentication check — The route exposes full API keys without verifying the request is from an authenticated user.

Other management routes (e.g., /api/db-backups/export/route.ts) use this pattern:

if (await isAuthRequired()) {
  if (!(await isAuthenticated(request))) {
    return NextResponse.json({ error: "Unauthorized" }, { status: 401 });
  }
}

Without this check, anyone who can access the dashboard (or if REQUIRE_API_KEY=false, anyone on the network) can retrieve full API keys by calling /api/keys/[id]/reveal.

try {
if (!isApiKeyRevealEnabled()) {
return NextResponse.json({ error: "API key reveal is disabled" }, { status: 403 });
}

const { id } = await params;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

In Next.js App Router route handlers, the params object is not a promise. The await here is unnecessary and likely a leftover from a testing setup. It should be removed for code clarity and correctness in a production environment.

Suggested change
const { id } = await params;
const { id } = params;

const key = await getApiKeyById(id);
Comment on lines +5 to +13

Copilot AI Mar 29, 2026

Copy link

Choose a reason for hiding this comment

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

The reveal handler doesn’t enforce management-route authentication. In production this may be covered by proxy middleware, but unit tests and any direct invocation will bypass that guard, and this endpoint returns a full secret. Consider calling requireManagementAuth(request) (or isAuthRequired + isAuthenticated) early and returning 401 when auth is required but missing.

Copilot uses AI. Check for mistakes.

if (!key || typeof key.key !== "string") {
return NextResponse.json({ error: "Key not found" }, { status: 404 });
}

return NextResponse.json({ key: key.key });

Copilot AI Mar 29, 2026

Copy link

Choose a reason for hiding this comment

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

This endpoint returns a sensitive secret but doesn’t set any cache-prevention headers. Add Cache-Control: no-store (and optionally Pragma: no-cache) on the 200 response to reduce the risk of intermediary/browser caching of the revealed API key.

Suggested change
return NextResponse.json({ key: key.key });
return NextResponse.json(
{ key: key.key },
{
headers: {
"Cache-Control": "no-store",
"Pragma": "no-cache",
},
},
);

Copilot uses AI. Check for mistakes.
} catch (error) {
console.log("Error revealing key:", error);
return NextResponse.json({ error: "Failed to reveal key" }, { status: 500 });
Comment on lines +4 to +22

Copilot AI Mar 29, 2026

Copy link

Choose a reason for hiding this comment

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

This new management API route uses ad-hoc NextResponse.json({ error: "..." }) payloads. The repo has canonical helpers in src/shared/utils/apiResponse.ts intended for management routes (structured { error: { code, message, ... } }). Using those helpers here would keep client error handling consistent.

Suggested change
// GET /api/keys/[id]/reveal - Reveal full API key for explicit copy actions
export async function GET(_request, { params }) {
try {
if (!isApiKeyRevealEnabled()) {
return NextResponse.json({ error: "API key reveal is disabled" }, { status: 403 });
}
const { id } = await params;
const key = await getApiKeyById(id);
if (!key || typeof key.key !== "string") {
return NextResponse.json({ error: "Key not found" }, { status: 404 });
}
return NextResponse.json({ key: key.key });
} catch (error) {
console.log("Error revealing key:", error);
return NextResponse.json({ error: "Failed to reveal key" }, { status: 500 });
import { errorResponse, successResponse } from "@/shared/utils/apiResponse";
// GET /api/keys/[id]/reveal - Reveal full API key for explicit copy actions
export async function GET(_request, { params }) {
try {
if (!isApiKeyRevealEnabled()) {
return errorResponse(
403,
"API_KEY_REVEAL_DISABLED",
"API key reveal is disabled",
);
}
const { id } = await params;
const key = await getApiKeyById(id);
if (!key || typeof key.key !== "string") {
return errorResponse(404, "API_KEY_NOT_FOUND", "Key not found");
}
return successResponse({ key: key.key });
} catch (error) {
console.log("Error revealing key:", error);
return errorResponse(500, "API_KEY_REVEAL_FAILED", "Failed to reveal key");

Copilot uses AI. Check for mistakes.
}
}
6 changes: 3 additions & 3 deletions src/app/api/keys/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,17 @@ import { getConsistentMachineId } from "@/shared/utils/machineId";
import { syncToCloud } from "@/lib/cloudSync";
import { createKeySchema } from "@/shared/validation/schemas";
import { isValidationFailure, validateBody } from "@/shared/validation/helpers";
import { isApiKeyRevealEnabled, maskStoredApiKey } from "@/lib/apiKeyExposure";

// GET /api/keys - List API keys
export async function GET() {
try {
const keys = await getApiKeys();
// Mask key values — users should never see full keys after creation
const maskedKeys = keys.map((k) => ({
...k,
key: typeof k.key === "string" ? k.key.slice(0, 8) + "****" + k.key.slice(-4) : null,
key: maskStoredApiKey(k.key),
}));
return NextResponse.json({ keys: maskedKeys });
return NextResponse.json({ keys: maskedKeys, allowKeyReveal: isApiKeyRevealEnabled() });
} catch (error) {
console.log("Error fetching keys:", error);
return NextResponse.json({ error: "Failed to fetch keys" }, { status: 500 });
Expand Down
13 changes: 13 additions & 0 deletions src/lib/apiKeyExposure.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
const ENABLED_VALUES = new Set(["1", "true", "yes", "on"]);

export function isApiKeyRevealEnabled(): boolean {
const raw = String(process.env.ALLOW_API_KEY_REVEAL || "")
.trim()
.toLowerCase();
return ENABLED_VALUES.has(raw);
}

export function maskStoredApiKey(key: unknown): string | null {
if (typeof key !== "string") return null;
return key.slice(0, 8) + "****" + key.slice(-4);

Copilot AI Mar 29, 2026

Copy link

Choose a reason for hiding this comment

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

maskStoredApiKey can fully disclose old/short API keys because the prefix (8) and suffix (4) slices overlap when key.length <= 12 (e.g., legacy format sk-{random8}), making the “masked” string reconstructable. Consider guarding on length and masking more aggressively for short keys (or choosing prefix/suffix lengths that never overlap).

Suggested change
return key.slice(0, 8) + "****" + key.slice(-4);
const str = key as string;
const len = str.length;
// For empty or very short keys, mask aggressively to avoid leaking the full value.
if (len === 0) {
return null;
}
if (len <= 4) {
return "****";
}
// For short keys, ensure prefix and suffix do not cover the entire string.
if (len <= 12) {
// Reveal only first and last character, mask everything in between.
return str[0] + "****" + str.slice(-1);
}
// For longer keys, keep existing behavior: show first 8 and last 4 characters.
return str.slice(0, 8) + "****" + str.slice(-4);

Copilot uses AI. Check for mistakes.
}
79 changes: 79 additions & 0 deletions tests/unit/api-key-reveal-route.test.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
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 TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-api-key-reveal-"));
process.env.DATA_DIR = TEST_DATA_DIR;
process.env.API_KEY_SECRET = "test-api-key-secret";

const core = await import("../../src/lib/db/core.ts");
const apiKeysDb = await import("../../src/lib/db/apiKeys.ts");
const listRoute = await import("../../src/app/api/keys/route.ts");
const revealRoute = await import("../../src/app/api/keys/[id]/reveal/route.ts");

const MACHINE_ID = "1234567890abcdef";

async function resetStorage() {
delete process.env.ALLOW_API_KEY_REVEAL;
core.resetDbInstance();
apiKeysDb.resetApiKeyState();
fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true });
fs.mkdirSync(TEST_DATA_DIR, { recursive: true });
}

function maskKey(key) {
return key.slice(0, 8) + "****" + key.slice(-4);
}

test.beforeEach(async () => {
await resetStorage();
});

test.after(async () => {
delete process.env.ALLOW_API_KEY_REVEAL;
core.resetDbInstance();
apiKeysDb.resetApiKeyState();
fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true });
});

test("GET /api/keys stays masked even when reveal is enabled", async () => {
process.env.ALLOW_API_KEY_REVEAL = "true";
const created = await apiKeysDb.createApiKey("Primary Key", MACHINE_ID);

const response = await listRoute.GET();
const body = await response.json();

assert.equal(response.status, 200);
assert.equal(body.allowKeyReveal, true);
assert.equal(Array.isArray(body.keys), true);
assert.equal(body.keys[0].key, maskKey(created.key));
});

test("GET /api/keys/[id]/reveal rejects requests when reveal is disabled", async () => {
const created = await apiKeysDb.createApiKey("Primary Key", MACHINE_ID);
const request = new Request(`http://localhost/api/keys/${created.id}/reveal`);

const response = await revealRoute.GET(request, {
params: Promise.resolve({ id: created.id }),
});
Comment on lines +58 to +60

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The params object passed to the route handler should be a plain object, not a promise. The route handler in Next.js receives { params } directly. Wrapping it in Promise.resolve in the test forces an unnecessary await in the production code. This should be corrected here and in the other test case.

  const response = await revealRoute.GET(request, {
    params: { id: created.id },
  });

const body = await response.json();

assert.equal(response.status, 403);
assert.equal(body.error, "API key reveal is disabled");
});

test("GET /api/keys/[id]/reveal returns the full key when reveal is enabled", async () => {
process.env.ALLOW_API_KEY_REVEAL = "true";
const created = await apiKeysDb.createApiKey("Primary Key", MACHINE_ID);
const request = new Request(`http://localhost/api/keys/${created.id}/reveal`);

const response = await revealRoute.GET(request, {
params: Promise.resolve({ id: created.id }),
});
Comment on lines +72 to +74

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Similar to the other test case, the params object should be a plain object, not a promise, to accurately reflect the Next.js route handler signature.

  const response = await revealRoute.GET(request, {
    params: { id: created.id },
  });

const body = await response.json();

assert.equal(response.status, 200);
assert.equal(body.key, created.key);
});