Repository navigation
feat(dashboard): inline show/hide toggle for API keys on API Manager page - #4505
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
Code Review
This pull request implements inline API key visibility toggling in the API Manager dashboard, allowing users to reveal or mask individual keys. It introduces client-side caching for revealed keys, updates the UI to default to masked keys, and adds utility functions (maskKey, toggleKeyVisibility) with accompanying unit tests. The review feedback suggests two key improvements: optimizing the copy button to use the client-side cache of revealed keys to prevent redundant network requests, and refactoring the unit tests to use Vitest instead of Node's built-in test runner to ensure consistency with the project's existing test suite.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| <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> |
There was a problem hiding this comment.
When the API key is already revealed and cached in revealedKeys, we can copy it directly from the client-side cache instead of making an unnecessary network request to /api/keys/{id}/reveal.
<button
onClick={async () => {
const cached = revealedKeys.get(key.id);
if (cached) {
await copy(cached, `existing_key_${key.id}`);
} else {
await 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>
| import { describe, it } from "node:test"; | ||
| import assert from "node:assert/strict"; | ||
| import { | ||
| maskKey, | ||
| toggleKeyVisibility, | ||
| } from "../../src/app/(dashboard)/dashboard/api-manager/apiManagerPageUtils.js"; | ||
|
|
||
| describe("maskKey", () => { | ||
| it("returns empty string when key is missing or empty", () => { | ||
| assert.equal(maskKey(""), ""); | ||
| assert.equal(maskKey(null), ""); | ||
| assert.equal(maskKey(undefined), ""); | ||
| }); | ||
|
|
||
| it("returns the key untouched when it fits the visible budget (<=8 chars)", () => { | ||
| assert.equal(maskKey("sk"), "sk"); | ||
| assert.equal(maskKey("sk-12345"), "sk-12345"); | ||
| }); | ||
|
|
||
| it("keeps the first 8 chars and appends an ellipsis when the key is longer", () => { | ||
| const full = "sk-or-1234567890abcdef"; | ||
| const masked = maskKey(full); | ||
| assert.equal(masked.startsWith("sk-or-12"), true); | ||
| assert.equal(masked.endsWith("..."), true); | ||
| // Must not leak the tail | ||
| assert.equal(masked.includes("90abcdef"), false); | ||
| }); | ||
| }); | ||
|
|
||
| describe("toggleKeyVisibility", () => { | ||
| it("adds an id when it is not present", () => { | ||
| const next = toggleKeyVisibility(new Set<string>(), "k1"); | ||
| assert.equal(next.has("k1"), true); | ||
| assert.equal(next.size, 1); | ||
| }); | ||
|
|
||
| it("removes an id when it is already present", () => { | ||
| const next = toggleKeyVisibility(new Set<string>(["k1", "k2"]), "k1"); | ||
| assert.equal(next.has("k1"), false); | ||
| assert.equal(next.has("k2"), true); | ||
| assert.equal(next.size, 1); | ||
| }); | ||
|
|
||
| it("returns a NEW Set (does not mutate the input — React state safety)", () => { | ||
| const input = new Set<string>(["k1"]); | ||
| const output = toggleKeyVisibility(input, "k2"); | ||
| assert.notEqual(output, input); | ||
| assert.equal(input.has("k2"), false); | ||
| assert.equal(output.has("k2"), true); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Since the project uses Vitest (as indicated by vitest.config.ts in the root), it is highly recommended to use Vitest's native APIs (describe, it, expect) instead of Node's built-in node:test and node:assert. This ensures consistent test execution, reporting, and coverage metrics across the entire test suite.
Here is how the test file can be refactored to use Vitest:
import { describe, it, expect } from "vitest";
import {
maskKey,
toggleKeyVisibility,
} from "../../src/app/(dashboard)/dashboard/api-manager/apiManagerPageUtils.js";
describe("maskKey", () => {
it("returns empty string when key is missing or empty", () => {
expect(maskKey("")).toBe("");
expect(maskKey(null)).toBe("");
expect(maskKey(undefined)).toBe("");
});
it("returns the key untouched when it fits the visible budget (<=8 chars)", () => {
expect(maskKey("sk")).toBe("sk");
expect(maskKey("sk-12345")).toBe("sk-12345");
});
it("keeps the first 8 chars and appends an ellipsis when the key is longer", () => {
const full = "sk-or-1234567890abcdef";
const masked = maskKey(full);
expect(masked.startsWith("sk-or-12")).toBe(true);
expect(masked.endsWith("...")).toBe(true);
expect(masked.includes("90abcdef")).toBe(false);
});
});
describe("toggleKeyVisibility", () => {
it("adds an id when it is not present", () => {
const next = toggleKeyVisibility(new Set<string>(), "k1");
expect(next.has("k1")).toBe(true);
expect(next.size).toBe(1);
});
it("removes an id when it is already present", () => {
const next = toggleKeyVisibility(new Set<string>(["k1", "k2"]), "k1");
expect(next.has("k1")).toBe(false);
expect(next.has("k2")).toBe(true);
expect(next.size).toBe(1);
});
it("returns a NEW Set (does not mutate the input — React state safety)", () => {
const input = new Set<string>(["k1"]);
const output = toggleKeyVisibility(input, "k2");
expect(output).not.toBe(input);
expect(input.has("k2")).toBe(false);
expect(output.has("k2")).toBe(true);
});
});690cf48 to
3bab3a0
Compare
…page
Each row in the API keys list now exposes an eye / eye-off button next to
the masked key. Clicking it lazy-fetches the full key via the existing
/api/keys/{id}/reveal endpoint (so the server-side policy gate is unchanged),
caches the plaintext client-side, and renders it inline. Clicking again
hides it. The toggle only appears when `allowKeyReveal` is true; when the
server policy disables reveal, the existing locked stub is preserved.
Implementation:
- Two pure helpers in apiManagerPageUtils.ts: `maskKey(s)` (keep first 8
chars + ellipsis) and `toggleKeyVisibility(prev, id)` (immutable Set
toggle, returns a fresh Set so React state setters always see a new
reference). 6-case TDD covers both, including the no-mutation contract.
- New `visibleKeys: Set<string>` and `revealedKeys: Map<string, string>`
state in ApiManagerPageClient.tsx. The reveal cache is cleared on
delete, so a recreated key cannot inherit a stale plaintext.
- Reuses the i18n keys `apiManager.showKey` / `apiManager.hideKey` that
already ship in all 42 locale catalogs — no new translation keys.
Inspired-by: decolua/9router#214
Co-authored-by: Thiên Toán <toanalien@gmail.com>
3bab3a0 to
1335fea
Compare
…page (cheaper-inference#4505) Rebuilt onto release/v3.8.33 (squash-base-stale). Integrated into release/v3.8.33.
Summary
Each row in the API keys list on the API Manager page now exposes an eye / eye-off button next to the masked key. Clicking it lazy-fetches the full key via the existing
/api/keys/{id}/revealendpoint (so the server-side policy gate is unchanged), caches the plaintext client-side, and renders it inline. Clicking again hides it. The toggle only appears whenallowKeyRevealis true — when the server policy disables reveal, the existing locked stub is preserved.What changed
apiManagerPageUtils.ts:maskKey(s)— keep the first 8 chars (provider prefix + a few entropy bits, e.g.sk-or-12...) and append an ellipsis. Returns""for missing input so the UI can render an empty<code>cleanly.toggleKeyVisibility(prev, id)— immutable Set toggle, returns a new Set so React state setters always see a fresh reference (no in-place mutation of state).ApiManagerPageClient.tsx:visibleKeys: Set<string>— which rows are currently revealed.revealedKeys: Map<string, string>— cached plaintext per id, populated on first reveal so re-toggling is instant.allowKeyRevealis true), and the existing copy button is preserved.apiManager.showKey/apiManager.hideKeyalready shipped in all 42 locale catalogs; no new translation keys.Test plan
tests/unit/api-manager-key-visibility.test.ts— 6 cases coveringmaskKeyedge cases (empty / short / long) andtoggleKeyVisibility(add / remove / no mutation).npm run typecheck:core— clean.npx eslinton the three edited files — 0 errors / 0 new warnings (3 pre-existing warnings unchanged).npm run check:any-budget:t11— PASS.api-manager-page-static.test.ts) — still green.Inspired-by: toanalien