mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-07-31 04:12:10 +03:00
fix(dashboard): stop double-masking already-masked API key in list (E2E 3/9 regression) (#4671)
Integrated into release/v3.8.37 — render server-masked key verbatim (drop redundant maskKey call). Note: release's maskKey already guards '****' (since v3.8.34), so this is a safe simplification; added a contract test pinning the **** passthrough invariant (2/2 green, would fail against the pre-guard maskKey = the historical double-mask bug).
This commit is contained in:
committed by
GitHub
parent
9f83aa9b93
commit
db77db0da3
@@ -16,7 +16,6 @@ import {
|
||||
computeApiKeyCounts,
|
||||
formatUsdCost,
|
||||
toLocalDateTimeInputValue,
|
||||
maskKey,
|
||||
toggleKeyVisibility,
|
||||
} from "./apiManagerPageUtils";
|
||||
import type { KeyStatus, KeyType } from "./apiManagerPageUtils";
|
||||
@@ -987,7 +986,7 @@ export default function ApiManagerPageClient() {
|
||||
<code className="text-sm text-text-muted font-mono truncate">
|
||||
{visibleKeys.has(key.id)
|
||||
? (revealedKeys.get(key.id) ?? key.key)
|
||||
: maskKey(key.key)}
|
||||
: key.key}
|
||||
</code>
|
||||
{allowKeyReveal ? (
|
||||
<>
|
||||
|
||||
39
tests/unit/api-manager-mask-key-passthrough.test.ts
Normal file
39
tests/unit/api-manager-mask-key-passthrough.test.ts
Normal file
@@ -0,0 +1,39 @@
|
||||
import test from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import { maskKey } from "../../src/app/(dashboard)/dashboard/api-manager/apiManagerPageUtils.ts";
|
||||
import { maskStoredApiKey } from "../../src/lib/apiKeyExposure.ts";
|
||||
|
||||
/**
|
||||
* Regression guard for #4671 (stop double-masking already-masked API keys).
|
||||
*
|
||||
* The keys-list endpoint (`/api/keys`) returns `key` already masked via
|
||||
* `maskStoredApiKey` → `slice(0,8) + "****" + slice(-4)`, which preserves the
|
||||
* last-4 suffix (e.g. `sk-live-****1002`). The dashboard list used to render that
|
||||
* value through `maskKey()` a SECOND time. Before maskKey gained its `"****"`
|
||||
* passthrough guard, the second pass truncated the value to `sk-live-...`,
|
||||
* dropping the suffix the user relies on to tell keys apart.
|
||||
*
|
||||
* #4671 drops the redundant `maskKey(key.key)` call and renders `key.key`
|
||||
* verbatim. These tests pin the invariant that makes that simplification safe:
|
||||
* an already-masked (`"****"`-bearing) value must round-trip unchanged through
|
||||
* maskKey, so `key.key` and `maskKey(key.key)` are equivalent for the real
|
||||
* server format. (At the pre-guard maskKey the second assertion would fail,
|
||||
* which is exactly the historical double-mask bug.)
|
||||
*/
|
||||
test("maskKey returns an already-masked (****) value verbatim — no double-mask", () => {
|
||||
const serverMasked = maskStoredApiKey("sk-live-abcdefgh-secret-tail-1002");
|
||||
assert.ok(serverMasked && serverMasked.includes("****"));
|
||||
// The suffix the user identifies the key by must survive.
|
||||
assert.ok(serverMasked.endsWith("1002"), "server mask must preserve the last-4 suffix");
|
||||
// The dashboard renders key.key directly; that must equal what maskKey would
|
||||
// have produced, i.e. the verbatim already-masked value (no truncation).
|
||||
assert.equal(maskKey(serverMasked), serverMasked);
|
||||
});
|
||||
|
||||
test("maskKey still masks a fully-revealed key (no '****') — first-pass behavior intact", () => {
|
||||
// A genuinely unmasked key (no "****") must still be shortened, so removing the
|
||||
// redundant second pass does not weaken masking of truly-revealed values.
|
||||
const full = "sk-live-abcdefgh-secret-tail-1002";
|
||||
assert.ok(!full.includes("****"));
|
||||
assert.equal(maskKey(full), "sk-live-...");
|
||||
});
|
||||
Reference in New Issue
Block a user