Skip to content

Commit 81477aa

Browse files
HXYerrorclaude
HXYerror
and
claude
committed
fix(admin-keys): address crew review findings for #32
R1 (blocking) — Renew button no longer disables debug: - Detail page now uses two separate forms: one POSTs debug_enabled=0 (disable), the other POSTs action=renew (refresh TTL). Route handler treats action=renew as setDebugEnabled(id, true) and audits key.debug_renew. Added a test that verifies debug_expires_at moves forward. R2 (blocking) — Server-side debug gate + CSP-clean scripts: - CSP default-src 'self' was blocking the inline confirmation modal AND the inline onclick/onsubmit handlers — the entire "two-click" debug gate was effectively bypassed. - Moved all admin keys JS to src/admin/assets/keys.js (loaded via <script src>), eliminating dangerouslySetInnerHTML and inline event handlers. Forms now use data-confirm attributes wired by addEventListener in keys.js. - Added a real server-side gate: POST /admin/keys/new and /:id/debug REJECT debug_enabled=1 unless body[debug_confirm]==yes. The modal sets the field to "yes" after acknowledgement; without JS debug cannot be enabled at all (safer default). R3 (blocking) — Stale-aware debug-active checks: - Added isDebugActive(row) helper: single source of truth combining debug_enabled, revoked_at, and debug_expires_at vs now. - countActiveDebugKeys() now filters by debug_expires_at > now in SQL, so the banner cannot lie between sweeps. - List/detail components use isDebugActive() everywhere. - Shortened sweeper interval from 5 min to 60 s. C3+C4 — Empty allowed_models + NaN page param: - parseAllowedModels returns {explicit, models}; empty-when-explicit is rejected with 400 instead of silently widening to ["*"]. - Hidden allowed_models_present=1 sentinel in the form distinguishes "no field" (default to ["*"]) from "field cleared" (reject). - parsePageParam guards against NaN: ?page=abc → page 1. C11 — Best-effort audit: - safeAudit() wraps audit() calls so a failed JSONL append (disk full, etc.) logs to consola.error instead of returning 500 to the user. The mutation is already durable in the DB at that point. C9 — Missing tests (10 new): - renew bumps debug_expires_at forward - server rejects debug_enabled=1 without debug_confirm - countActiveDebugKeys + isDebugActive exclude TTL-expired keys - GET /admin/keys/<non-uuid> → 404 (no DB hit) - POST /admin/keys/<non-uuid>/revoke → 404 - Label >200 chars rejected - Label with <script> rendered as escaped text on list page - Empty allowed_models rejected with explicit error - Missing allowed_models field still defaults to ["*"] - GET /admin/keys?page=abc → page 1 (no NaN) - GET /admin/keys/created?flash=<unknown> → 410 with explicit error - Static asset /admin/assets/keys.js served with javascript MIME Also: per C2, the consumeFlash miss now renders 410 Gone with an explicit error message instead of silently redirecting to /admin/keys. 416/416 tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 7277dea commit 81477aa

9 files changed

Lines changed: 604 additions & 165 deletions

File tree

‎eslint.config.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,4 +4,5 @@ export default config({
44
prettier: {
55
plugins: ["prettier-plugin-packagejson"],
66
},
7+
ignores: ["src/admin/assets/**/*.js"],
78
})

‎src/admin/assets/keys.js‎

Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,137 @@
1+
// Copilot API Admin — keys management interactivity
2+
// Loaded as an external script under default-src 'self' CSP.
3+
// No inline event handlers — all DOM wiring via addEventListener.
4+
(function () {
5+
"use strict"
6+
7+
// ---------------------------------------------------------------------------
8+
// Debug-checkbox confirmation modal (new key form)
9+
// ---------------------------------------------------------------------------
10+
function wireNewKeyDebugModal() {
11+
var cb = document.getElementById("debug-checkbox")
12+
var modal = document.getElementById("debug-modal")
13+
var confirmBtn = document.getElementById("debug-confirm")
14+
var cancelBtn = document.getElementById("debug-cancel")
15+
var hiddenConfirm = document.getElementById("debug-confirm-field")
16+
if (!cb || !modal || !confirmBtn || !cancelBtn || !hiddenConfirm) return
17+
var confirmed = false
18+
19+
cb.addEventListener("change", function () {
20+
if (cb.checked && !confirmed) {
21+
cb.checked = false
22+
modal.style.display = "flex"
23+
} else if (!cb.checked) {
24+
confirmed = false
25+
hiddenConfirm.value = ""
26+
}
27+
})
28+
29+
confirmBtn.addEventListener("click", function () {
30+
confirmed = true
31+
cb.checked = true
32+
hiddenConfirm.value = "yes"
33+
modal.style.display = "none"
34+
})
35+
36+
cancelBtn.addEventListener("click", function () {
37+
confirmed = false
38+
cb.checked = false
39+
hiddenConfirm.value = ""
40+
modal.style.display = "none"
41+
})
42+
}
43+
44+
// ---------------------------------------------------------------------------
45+
// Debug-enable button confirmation modal (detail page)
46+
// ---------------------------------------------------------------------------
47+
function wireDetailDebugModal() {
48+
var btn = document.getElementById("debug-btn")
49+
var modal = document.getElementById("debug-modal")
50+
var form = document.getElementById("debug-form")
51+
var confirmBtn = document.getElementById("debug-confirm")
52+
var cancelBtn = document.getElementById("debug-cancel")
53+
var hiddenConfirm = document.getElementById("debug-confirm-field")
54+
if (!btn || !modal || !form || !confirmBtn || !cancelBtn || !hiddenConfirm)
55+
return
56+
57+
btn.addEventListener("click", function (e) {
58+
e.preventDefault()
59+
modal.style.display = "flex"
60+
})
61+
62+
confirmBtn.addEventListener("click", function () {
63+
hiddenConfirm.value = "yes"
64+
modal.style.display = "none"
65+
form.submit()
66+
})
67+
68+
cancelBtn.addEventListener("click", function () {
69+
modal.style.display = "none"
70+
})
71+
}
72+
73+
// ---------------------------------------------------------------------------
74+
// Revoke-confirm dialogs (replaces inline onsubmit="return confirm(...)")
75+
// ---------------------------------------------------------------------------
76+
function wireRevokeForms() {
77+
var forms = document.querySelectorAll("form[data-confirm]")
78+
Array.prototype.forEach.call(forms, function (form) {
79+
form.addEventListener("submit", function (e) {
80+
var msg = form.getAttribute("data-confirm") || "Are you sure?"
81+
if (!window.confirm(msg)) e.preventDefault()
82+
})
83+
})
84+
}
85+
86+
// ---------------------------------------------------------------------------
87+
// Created-page: copy button + "I have copied" gate + beforeunload guard
88+
// ---------------------------------------------------------------------------
89+
function wireKeyCreatedPage() {
90+
var keyEl = document.getElementById("plain-key")
91+
var copyBtn = document.getElementById("copy-btn")
92+
var gate = document.getElementById("copied-gate")
93+
var link = document.getElementById("continue-link")
94+
if (!keyEl || !copyBtn || !gate || !link) return
95+
96+
copyBtn.addEventListener("click", function () {
97+
var text = keyEl.textContent || ""
98+
if (navigator.clipboard && navigator.clipboard.writeText) {
99+
navigator.clipboard.writeText(text).then(
100+
function () {
101+
copyBtn.textContent = "Copied!"
102+
},
103+
function () {
104+
copyBtn.textContent = "Copy failed"
105+
},
106+
)
107+
}
108+
})
109+
110+
gate.addEventListener("change", function () {
111+
link.style.pointerEvents = gate.checked ? "" : "none"
112+
link.style.opacity = gate.checked ? "1" : "0.5"
113+
})
114+
115+
window.addEventListener("beforeunload", function (e) {
116+
if (!gate.checked) {
117+
e.preventDefault()
118+
e.returnValue = "Have you copied your API key?"
119+
}
120+
})
121+
}
122+
123+
// Run all wirings once the DOM is ready.
124+
if (document.readyState === "loading") {
125+
document.addEventListener("DOMContentLoaded", function () {
126+
wireNewKeyDebugModal()
127+
wireDetailDebugModal()
128+
wireRevokeForms()
129+
wireKeyCreatedPage()
130+
})
131+
} else {
132+
wireNewKeyDebugModal()
133+
wireDetailDebugModal()
134+
wireRevokeForms()
135+
wireKeyCreatedPage()
136+
}
137+
})()

‎src/admin/keys/detail.tsx‎

Lines changed: 32 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ import type { FC } from "hono/jsx"
33

44
import type { KeyRow } from "~/services/keys"
55

6+
import { isDebugActive } from "~/services/keys"
7+
68
import { fmtDate, fmtModels } from "./list"
79

810
// ---------------------------------------------------------------------------
@@ -13,7 +15,7 @@ const KeyMeta: FC<{ row: KeyRow; expiresStr: string }> = ({
1315
row,
1416
expiresStr,
1517
}) => {
16-
const debugOn = row.debug_enabled === 1
18+
const debugOn = isDebugActive(row)
1719
return (
1820
<div class="key-meta">
1921
<dl>
@@ -96,37 +98,35 @@ const DebugEnabledControls: FC<{
9698
Debug is <strong>ON</strong>. Traces persist in plaintext. Retention:{" "}
9799
{tracesDays} days.{expiresStr}
98100
</p>
99-
<form method="post" action={`/admin/keys/${row.id}/debug`}>
101+
{/* Disable form */}
102+
<form
103+
method="post"
104+
action={`/admin/keys/${row.id}/debug`}
105+
style="display:inline"
106+
>
100107
<input type="hidden" name="csrf_token" value={csrfToken} />
101108
<input type="hidden" name="debug_enabled" value="0" />
102109
<button type="submit" class="btn btn-sm">
103110
Disable Debug
104111
</button>
105-
<button
106-
type="submit"
107-
name="renew"
108-
value="1"
109-
class="btn btn-sm btn-warning"
110-
>
112+
</form>
113+
{/* Renew form: separate POST with action=renew so the handler bumps TTL
114+
instead of clearing it (the previous shared form silently disabled). */}
115+
<form
116+
method="post"
117+
action={`/admin/keys/${row.id}/debug`}
118+
style="display:inline"
119+
>
120+
<input type="hidden" name="csrf_token" value={csrfToken} />
121+
<input type="hidden" name="action" value="renew" />
122+
<input type="hidden" name="debug_confirm" value="yes" />
123+
<button type="submit" class="btn btn-sm btn-warning">
111124
Renew 24h TTL
112125
</button>
113126
</form>
114127
</>
115128
)
116129

117-
const DEBUG_ENABLE_SCRIPT = `
118-
(function(){
119-
var btn = document.getElementById('debug-btn');
120-
var modal = document.getElementById('debug-modal');
121-
var form = document.getElementById('debug-form');
122-
var confirmBtn = document.getElementById('debug-confirm');
123-
var cancelBtn = document.getElementById('debug-cancel');
124-
btn.addEventListener('click', function(e){ e.preventDefault(); modal.style.display='flex'; });
125-
confirmBtn.addEventListener('click', function(){ modal.style.display='none'; form.submit(); });
126-
cancelBtn.addEventListener('click', function(){ modal.style.display='none'; });
127-
})();
128-
`.trim()
129-
130130
const DebugDisabledControls: FC<{
131131
row: KeyRow
132132
csrfToken: string
@@ -158,14 +158,18 @@ const DebugDisabledControls: FC<{
158158
<form method="post" action={`/admin/keys/${row.id}/debug`} id="debug-form">
159159
<input type="hidden" name="csrf_token" value={csrfToken} />
160160
<input type="hidden" name="debug_enabled" value="1" />
161+
{/* keys.js sets this to "yes" after the modal is acknowledged.
162+
The server REJECTS debug_enabled=1 without it. */}
163+
<input
164+
type="hidden"
165+
id="debug-confirm-field"
166+
name="debug_confirm"
167+
value=""
168+
/>
161169
<button type="submit" id="debug-btn" class="btn btn-warning">
162170
Enable Debug (24h)
163171
</button>
164172
</form>
165-
<script
166-
// biome-ignore lint/security/noDangerouslySetInnerHtml: intentional inline script for admin-only page
167-
dangerouslySetInnerHTML={{ __html: DEBUG_ENABLE_SCRIPT }}
168-
/>
169173
</>
170174
)
171175

@@ -178,7 +182,7 @@ const RevokeSection: FC<{ row: KeyRow; csrfToken: string }> = ({
178182
<form
179183
method="post"
180184
action={`/admin/keys/${row.id}/revoke`}
181-
onsubmit="return confirm('Revoke this key? This cannot be undone.')"
185+
data-confirm="Revoke this key? This cannot be undone."
182186
>
183187
<input type="hidden" name="csrf_token" value={csrfToken} />
184188
<button type="submit" class="btn btn-danger">
@@ -208,7 +212,7 @@ export const KeyDetail: FC<KeyDetailProps> = ({
208212
success,
209213
}) => {
210214
const isRevoked = row.revoked_at !== null
211-
const debugOn = row.debug_enabled === 1
215+
const debugOn = isDebugActive(row)
212216
const idSuffix = row.id.slice(-8)
213217
const expiresStr =
214218
row.debug_expires_at ?
@@ -264,6 +268,7 @@ export const KeyDetail: FC<KeyDetailProps> = ({
264268
<RevokeSection row={row} csrfToken={csrfToken} />
265269
</>
266270
)}
271+
<script src="/admin/assets/keys.js" />
267272
</div>
268273
)
269274
}

‎src/admin/keys/list.tsx‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ import type { FC } from "hono/jsx"
33

44
import type { KeyRow } from "~/services/keys"
55

6+
import { isDebugActive } from "~/services/keys"
7+
68
// ---------------------------------------------------------------------------
79
// Shared helpers
810
// ---------------------------------------------------------------------------
@@ -91,6 +93,7 @@ export const KeyList: FC<KeyListProps> = ({
9193
</a>
9294
)}
9395
</div>
96+
<script src="/admin/assets/keys.js" />
9497
</div>
9598
)
9699
}
@@ -106,7 +109,7 @@ interface KeyRowProps {
106109

107110
const KeyRow: FC<KeyRowProps> = ({ row, csrfToken }) => {
108111
const isRevoked = row.revoked_at !== null
109-
const debugOn = row.debug_enabled === 1
112+
const debugOn = isDebugActive(row)
110113
const idSuffix = row.id.slice(-8)
111114
const expiresStr =
112115
row.debug_expires_at ? ` (exp ${fmtDate(row.debug_expires_at)})` : ""
@@ -148,7 +151,7 @@ const KeyRow: FC<KeyRowProps> = ({ row, csrfToken }) => {
148151
method="post"
149152
action={`/admin/keys/${row.id}/revoke`}
150153
class="inline-form"
151-
onsubmit="return confirm('Revoke this key? This cannot be undone.')"
154+
data-confirm="Revoke this key? This cannot be undone."
152155
>
153156
<input type="hidden" name="csrf_token" value={csrfToken} />
154157
<button type="submit" class="btn btn-sm btn-danger">

0 commit comments

Comments
 (0)