mirror of
https://github.com/dtzp555-max/ocp.git
synced 2026-07-21 21:15:09 +00:00
The dashboard built table rows via template-literal innerHTML, interpolating
DB-sourced strings (key names, usage rows) with no HTML escaping, and an
onclick="revokeKeyUI('${k.name}')" sink a single quote could break out of. Key
names were unvalidated at creation. Admin-gated (self-XSS today), but a real
unescaped-sink gap that becomes cross-user if key creation is ever delegated.
- dashboard.html: added escapeHtml() and applied it to every DB/string-sourced
interpolation in refreshUsage and refreshKeys (key_name, name, keyPreview,
created_at, last_request, model). Replaced the inline-onclick revoke button with
a data-revoke attribute + addEventListener, so a name can never break out into an
event-handler string. (model is attacker-pickable via the request body, so its
escaping is the load-bearing one.)
- server.mjs: POST /api/keys now rejects names not matching /^[A-Za-z0-9 ._-]{1,64}$/
before createKey() — defense-in-depth so a <script>/quote name can never reach the
DB. Creation-only; existing keys unaffected; the default key-${Date.now()} passes.
Noted (out of scope, optional follow-up): the status/plan summary cards render
trusted server/Anthropic-upstream values unescaped — non-user-controlled, so not part
of this stored-XSS fix.
ALIGNMENT.md: the server.mjs change is proxy-policy input validation with no Anthropic
operation forwarded, so a cli.js citation is N/A under Rule 2. No blacklisted tokens or
port literals introduced; alignment.yml passes.
Independent fresh-context reviewer (opus): APPROVE WITH MINOR (Iron Rule 10) — verified
escapeHtml correctness, full sink coverage (incl. the attacker-pickable model field and
the data-revoke attribute), revoke round-trip via getAttribute, the anchored/bounded
key-name regex running before createKey, and that createKey has no other unvalidated
caller. Both minors non-blocking (trusted status-card escaping; this commit-body note).
Closes #114.
Co-authored-by: dtzp555 <dtzp555@gmail.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
+16
-9
@@ -132,6 +132,10 @@ function fmtChars(n) {
|
|||||||
return n > 1000 ? (n/1000).toFixed(0) + "K" : String(n);
|
return n > 1000 ? (n/1000).toFixed(0) + "K" : String(n);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function escapeHtml(s) {
|
||||||
|
return String(s ?? "").replace(/[&<>"']/g, c => ({ "&": "&", "<": "<", ">": ">", '"': """, "'": "'" }[c]));
|
||||||
|
}
|
||||||
|
|
||||||
function barColor(pct) {
|
function barColor(pct) {
|
||||||
if (pct >= 80) return "bar-red";
|
if (pct >= 80) return "bar-red";
|
||||||
if (pct >= 50) return "bar-amber";
|
if (pct >= 50) return "bar-amber";
|
||||||
@@ -181,21 +185,21 @@ async function refreshUsage() {
|
|||||||
const tbody = document.querySelector("#key-usage-table tbody");
|
const tbody = document.querySelector("#key-usage-table tbody");
|
||||||
tbody.innerHTML = (data.byKey || []).map(k => `
|
tbody.innerHTML = (data.byKey || []).map(k => `
|
||||||
<tr>
|
<tr>
|
||||||
<td>${k.key_name}</td>
|
<td>${escapeHtml(k.key_name)}</td>
|
||||||
<td>${k.requests}</td>
|
<td>${k.requests}</td>
|
||||||
<td>${k.successes}</td>
|
<td>${k.successes}</td>
|
||||||
<td>${k.errors}</td>
|
<td>${k.errors}</td>
|
||||||
<td>${fmtTime(k.avg_elapsed_ms)}</td>
|
<td>${fmtTime(k.avg_elapsed_ms)}</td>
|
||||||
<td class="mono">${k.last_request || '-'}</td>
|
<td class="mono">${escapeHtml(k.last_request || '-')}</td>
|
||||||
</tr>
|
</tr>
|
||||||
`).join("") || '<tr><td colspan="6" style="color:#475569">No usage data yet</td></tr>';
|
`).join("") || '<tr><td colspan="6" style="color:#475569">No usage data yet</td></tr>';
|
||||||
|
|
||||||
const rtbody = document.querySelector("#recent-table tbody");
|
const rtbody = document.querySelector("#recent-table tbody");
|
||||||
rtbody.innerHTML = (data.recent || []).slice(0, 20).map(r => `
|
rtbody.innerHTML = (data.recent || []).slice(0, 20).map(r => `
|
||||||
<tr>
|
<tr>
|
||||||
<td class="mono">${r.created_at?.slice(11, 19) || '?'}</td>
|
<td class="mono">${escapeHtml(r.created_at?.slice(11, 19) || '?')}</td>
|
||||||
<td>${r.key_name}</td>
|
<td>${escapeHtml(r.key_name)}</td>
|
||||||
<td>${r.model}</td>
|
<td>${escapeHtml(r.model)}</td>
|
||||||
<td>${fmtChars(r.prompt_chars)}</td>
|
<td>${fmtChars(r.prompt_chars)}</td>
|
||||||
<td>${fmtChars(r.response_chars)}</td>
|
<td>${fmtChars(r.response_chars)}</td>
|
||||||
<td>${fmtTime(r.elapsed_ms)}</td>
|
<td>${fmtTime(r.elapsed_ms)}</td>
|
||||||
@@ -216,13 +220,16 @@ async function refreshKeys() {
|
|||||||
const tbody = document.querySelector("#keys-table tbody");
|
const tbody = document.querySelector("#keys-table tbody");
|
||||||
tbody.innerHTML = (data.keys || []).map(k => `
|
tbody.innerHTML = (data.keys || []).map(k => `
|
||||||
<tr>
|
<tr>
|
||||||
<td>${k.name}</td>
|
<td>${escapeHtml(k.name)}</td>
|
||||||
<td class="mono">${k.keyPreview}</td>
|
<td class="mono">${escapeHtml(k.keyPreview)}</td>
|
||||||
<td class="mono">${k.created_at}</td>
|
<td class="mono">${escapeHtml(k.created_at)}</td>
|
||||||
<td><span class="tag ${k.revoked ? 'tag-err' : 'tag-ok'}">${k.revoked ? 'revoked' : 'active'}</span></td>
|
<td><span class="tag ${k.revoked ? 'tag-err' : 'tag-ok'}">${k.revoked ? 'revoked' : 'active'}</span></td>
|
||||||
<td>${k.revoked ? '' : `<button class="btn btn-sm btn-danger" onclick="revokeKeyUI('${k.name}')">Revoke</button>`}</td>
|
<td>${k.revoked ? '' : `<button class="btn btn-sm btn-danger" data-revoke="${escapeHtml(k.name)}">Revoke</button>`}</td>
|
||||||
</tr>
|
</tr>
|
||||||
`).join("");
|
`).join("");
|
||||||
|
tbody.querySelectorAll("button[data-revoke]").forEach(btn =>
|
||||||
|
btn.addEventListener("click", () => revokeKeyUI(btn.getAttribute("data-revoke")))
|
||||||
|
);
|
||||||
} catch(e) { /* not admin */ }
|
} catch(e) { /* not admin */ }
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -2048,6 +2048,9 @@ const server = createServer(async (req, res) => {
|
|||||||
let parsed;
|
let parsed;
|
||||||
try { parsed = JSON.parse(body); } catch { return jsonResponse(res, 400, { error: "Invalid JSON" }); }
|
try { parsed = JSON.parse(body); } catch { return jsonResponse(res, 400, { error: "Invalid JSON" }); }
|
||||||
const name = parsed.name || `key-${Date.now()}`;
|
const name = parsed.name || `key-${Date.now()}`;
|
||||||
|
if (!/^[A-Za-z0-9 ._-]{1,64}$/.test(name)) {
|
||||||
|
return jsonResponse(res, 400, { error: { message: "Invalid key name: 1-64 chars of letters, digits, space, dot, underscore, hyphen", type: "invalid_request_error" } });
|
||||||
|
}
|
||||||
const newKey = createKey(name);
|
const newKey = createKey(name);
|
||||||
return jsonResponse(res, 201, newKey);
|
return jsonResponse(res, 201, newKey);
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1743,6 +1743,55 @@ test("models.json aliases.sonnet === 'claude-sonnet-4-6' (default-request-model
|
|||||||
assert.equal(_spotModels.aliases.sonnet, "claude-sonnet-4-6");
|
assert.equal(_spotModels.aliases.sonnet, "claude-sonnet-4-6");
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// ── escapeHtml + key-name validator (issue #114) ────────────────────────────
|
||||||
|
// Replicated verbatim from dashboard.html so tests run without a browser.
|
||||||
|
function escapeHtml(s) {
|
||||||
|
return String(s ?? "").replace(/[&<>"']/g, c => ({ "&": "&", "<": "<", ">": ">", '"': """, "'": "'" }[c]));
|
||||||
|
}
|
||||||
|
const KEY_NAME_RE = /^[A-Za-z0-9 ._-]{1,64}$/;
|
||||||
|
|
||||||
|
console.log("\nescapeHtml (issue #114):");
|
||||||
|
|
||||||
|
test("escapeHtml: XSS payload → <img not <img", () => {
|
||||||
|
const out = escapeHtml('<img src=x onerror=alert(1)>');
|
||||||
|
assert.ok(out.includes("<img"), `expected <img in: ${out}`);
|
||||||
|
assert.ok(!out.includes("<img"), `expected no raw <img in: ${out}`);
|
||||||
|
});
|
||||||
|
|
||||||
|
test("escapeHtml: single-quote, double-quote, ampersand all escaped", () => {
|
||||||
|
assert.equal(escapeHtml("a'b\"c&d"), "a'b"c&d");
|
||||||
|
});
|
||||||
|
|
||||||
|
test("escapeHtml: null → empty string", () => {
|
||||||
|
assert.equal(escapeHtml(null), "");
|
||||||
|
});
|
||||||
|
|
||||||
|
console.log("\nKey-name validator (issue #114):");
|
||||||
|
|
||||||
|
test("KEY_NAME_RE: 'wife-laptop' → valid", () => {
|
||||||
|
assert.ok(KEY_NAME_RE.test("wife-laptop"));
|
||||||
|
});
|
||||||
|
|
||||||
|
test("KEY_NAME_RE: 'key-1700000000000' → valid", () => {
|
||||||
|
assert.ok(KEY_NAME_RE.test("key-1700000000000"));
|
||||||
|
});
|
||||||
|
|
||||||
|
test("KEY_NAME_RE: '<script>' → invalid", () => {
|
||||||
|
assert.ok(!KEY_NAME_RE.test("<script>"));
|
||||||
|
});
|
||||||
|
|
||||||
|
test("KEY_NAME_RE: \"a'); DROP\" → invalid", () => {
|
||||||
|
assert.ok(!KEY_NAME_RE.test("a'); DROP"));
|
||||||
|
});
|
||||||
|
|
||||||
|
test("KEY_NAME_RE: empty string → invalid", () => {
|
||||||
|
assert.ok(!KEY_NAME_RE.test(""));
|
||||||
|
});
|
||||||
|
|
||||||
|
test("KEY_NAME_RE: 65-char string → invalid", () => {
|
||||||
|
assert.ok(!KEY_NAME_RE.test("x".repeat(65)));
|
||||||
|
});
|
||||||
|
|
||||||
// ── Cleanup ──
|
// ── Cleanup ──
|
||||||
closeDb();
|
closeDb();
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user