David/HERMES-GIELIE
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix(#4771): block duplicate approval submits

Browse source
This commit is contained in:
Rod Boev 2026-06-25 08:04:43 -04:00 committed by nesquena-hermes
parent 0b12b3b1fb
commit 7d617d91f7
2 changed files with 173 additions and 14 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

45
static/messages.js
View file

@ -5361,6 +5361,7 @@ function hideApprovalCard(force=false) {
let _approvalSessionId = null;
let _approvalCurrentId = null; // approval_id of the card currently shown
let _approvalPendingBySession = new Map();
let _approvalResponding = null;
const _DISMISSED_APPROVALS_KEY = 'hermes_dismissed_approvals';
@ -5449,6 +5450,27 @@ function _renderPendingApprovalForActiveSession() {
if (entry) showApprovalCard(entry.pending, entry.pendingCount);
}
function _approvalResponseMatches(sid, approvalId) {
return !!(
_approvalResponding &&
_approvalResponding.sid === sid &&
(_approvalResponding.approvalId || null) === (approvalId || null)
);
}
function _setApprovalControlsDisabled(choice, disabled) {
["approvalBtnOnce","approvalBtnSession","approvalBtnAlways","approvalBtnDeny"].forEach(id => {
const b = $(id);
if (!b) return;
b.disabled = !!disabled;
if (disabled && choice && b.id === "approvalBtn" + choice.charAt(0).toUpperCase() + choice.slice(1)) {
b.classList.add("loading");
} else {
b.classList.remove("loading");
}
});
}
function showApprovalForSession(sid, pending, pendingCount) {
if (!pending) return;
pending._session_id = sid;
@ -5487,10 +5509,11 @@ function showApprovalCard(pending, pendingCount) {
// approval's collapsed state, which would hide its command + action buttons. (#3515)
card.classList.remove("collapsed");
}
// Re-enable buttons in case a previous approval disabled them
["approvalBtnOnce","approvalBtnSession","approvalBtnAlways","approvalBtnDeny"].forEach(id => {
const b = $(id); if (b) { b.disabled = false; b.classList.remove("loading"); }
});
const responding = _approvalResponseMatches(sid, _approvalCurrentId);
_setApprovalControlsDisabled(
responding ? _approvalResponding.choice : null,
responding,
);
card.classList.add("visible");
_syncApprovalCollapseButton(card);
_syncApprovalTranscriptSpace(card, {immediate: true});
@ -5560,9 +5583,8 @@ function _syncApprovalTranscriptSpace(card, opts) {
}
function _restoreFailedApprovalResponse(sid, errMsg) {
["approvalBtnOnce","approvalBtnSession","approvalBtnAlways","approvalBtnDeny"].forEach(id => {
const b = $(id); if (b) { b.disabled = false; b.classList.remove("loading"); }
});
_approvalResponding = null;
_setApprovalControlsDisabled(null, false);
if (_approvalPromptBelongsToActiveSession(sid)) _renderPendingApprovalForActiveSession();
if (typeof showToast === "function") showToast(errMsg, 5000);
if (typeof setStatus === "function") setStatus(errMsg);
@ -5581,18 +5603,17 @@ async function respondApproval(choice) {
const sid = _approvalSessionId || (S.session && S.session.session_id);
if (!sid) return;
const approvalId = _approvalCurrentId;
if (_approvalResponseMatches(sid, approvalId)) return;
_unmarkApprovalDismissed(sid, approvalId);
// Disable all buttons immediately to prevent double-submit
["approvalBtnOnce","approvalBtnSession","approvalBtnAlways","approvalBtnDeny"].forEach(id => {
const b = $(id);
if (b) { b.disabled = true; if (b.id === "approvalBtn" + choice.charAt(0).toUpperCase() + choice.slice(1)) b.classList.add("loading"); }
});
_approvalResponding = {sid, approvalId: approvalId || null, choice};
_setApprovalControlsDisabled(choice, true);
try {
const result = await api("/api/approval/respond", {
method: "POST",
body: JSON.stringify({ session_id: sid, choice, approval_id: approvalId })
});
if (result && result.ok) {
_approvalResponding = null;
const pendingEntry = _approvalPendingBySession.get(sid);
const samePending = !!(pendingEntry && pendingEntry.pending && (pendingEntry.pending.approval_id || null) === (approvalId || null));
if (_approvalSessionId === sid && _approvalCurrentId === approvalId) {

142
tests/test_issue4771_gateway_approval_failure_ui.py
View file

@ -60,6 +60,8 @@ def _run_failure_case(api_js: str) -> dict:
_extract_fn(MESSAGES_JS, "_rememberApprovalPending"),
_extract_fn(MESSAGES_JS, "_clearApprovalPendingForSession"),
_extract_fn(MESSAGES_JS, "_renderPendingApprovalForActiveSession"),
_extract_fn(MESSAGES_JS, "_approvalResponseMatches"),
_extract_fn(MESSAGES_JS, "_setApprovalControlsDisabled"),
_extract_fn(MESSAGES_JS, "showApprovalCard"),
_extract_fn(MESSAGES_JS, "_restoreFailedApprovalResponse"),
_extract_fn(MESSAGES_JS, "respondApproval", prefix="async function "),
@ -82,6 +84,7 @@ let renderCalls = 0;
let _approvalSessionId = 'sess-1';
let _approvalCurrentId = 'appr-1';
let _approvalPendingBySession = new Map();
let _approvalResponding = null;
let _approvalSignature = '';
let _approvalVisibleSince = 0;
let _approvalHideTimer = null;
@ -155,11 +158,11 @@ const pending = {{
}};
_approvalPendingBySession.set('sess-1', {{ pending, pendingCount: 1 }});
showApprovalCard(pending, 1);
const realRenderPendingApprovalForActiveSession = _renderPendingApprovalForActiveSession;
renderCalls = 0;
_renderPendingApprovalForActiveSession = function() {{
renderCalls += 1;
const entry = _approvalPendingBySession.get('sess-1');
if (entry) showApprovalCard(entry.pending, entry.pendingCount);
return realRenderPendingApprovalForActiveSession();
}};
respondApproval('once').then(() => {{
output.sessionId = _approvalSessionId;
@ -220,3 +223,138 @@ def test_rejected_ok_false_approval_keeps_card_and_reenables_buttons():
assert out["cardVisible"] is True
assert "Approval response not accepted for this session." == out["toast"]
assert "Approval response not accepted for this session." == out["status"]
def test_poll_rerender_keeps_inflight_buttons_disabled_and_blocks_duplicates():
helpers = "\n".join(
[
_extract_fn(MESSAGES_JS, "_approvalDismissKey"),
_extract_fn(MESSAGES_JS, "_getDismissedApprovals"),
_extract_fn(MESSAGES_JS, "_isApprovalDismissed"),
_extract_fn(MESSAGES_JS, "_unmarkApprovalDismissed"),
_extract_fn(MESSAGES_JS, "_promptActiveSessionId"),
_extract_fn(MESSAGES_JS, "_approvalPromptBelongsToActiveSession"),
_extract_fn(MESSAGES_JS, "_rememberApprovalPending"),
_extract_fn(MESSAGES_JS, "_clearApprovalPendingForSession"),
_extract_fn(MESSAGES_JS, "_renderPendingApprovalForActiveSession"),
_extract_fn(MESSAGES_JS, "_approvalResponseMatches"),
_extract_fn(MESSAGES_JS, "_setApprovalControlsDisabled"),
_extract_fn(MESSAGES_JS, "showApprovalCard"),
_extract_fn(MESSAGES_JS, "_restoreFailedApprovalResponse"),
_extract_fn(MESSAGES_JS, "respondApproval", prefix="async function "),
]
)
script = f"""
const output = {{}};
const _store = {{}};
const localStorage = {{
getItem: key => Object.prototype.hasOwnProperty.call(_store, key) ? _store[key] : null,
setItem: (key, value) => {{ _store[key] = String(value); }},
}};
const S = {{ session: {{ session_id: 'sess-1' }} }};
const _DISMISSED_APPROVALS_KEY = 'hermes_dismissed_approvals';
let _approvalSessionId = 'sess-1';
let _approvalCurrentId = 'appr-1';
let _approvalPendingBySession = new Map();
let _approvalResponding = null;
let _approvalSignature = '';
let _approvalVisibleSince = 0;
let _approvalHideTimer = null;
const _clarifyPendingBySession = new Map();
let resolveApi;
let apiCalls = 0;
const buttons = new Map();
function makeButton(id) {{
return {{
id,
disabled: false,
classList: {{
values: new Set(),
add(value) {{ this.values.add(value); }},
remove(value) {{ this.values.delete(value); }},
contains(value) {{ return this.values.has(value); }},
}},
}};
}}
['approvalBtnOnce','approvalBtnSession','approvalBtnAlways','approvalBtnDeny'].forEach(id => buttons.set(id, makeButton(id)));
const card = {{
classList: {{
values: new Set(['visible']),
add(value) {{ this.values.add(value); }},
remove(value) {{ this.values.delete(value); }},
contains(value) {{ return this.values.has(value); }},
toggle(value, force) {{
if (force === undefined) {{
if (this.values.has(value)) this.values.delete(value);
else this.values.add(value);
}} else if (force) this.values.add(value);
else this.values.delete(value);
}},
}},
querySelector() {{ return null; }},
}};
const approvalDesc = {{ textContent: '' }};
const approvalCmd = {{ textContent: '' }};
const approvalCounter = {{ textContent: '', style: {{ display: '' }} }};
const msg = {{}};
const document = {{ activeElement: null }};
const elements = {{
approvalCard: card,
approvalDesc,
approvalCmd,
approvalCounter,
approvalBtnOnce: buttons.get('approvalBtnOnce'),
approvalBtnSession: buttons.get('approvalBtnSession'),
approvalBtnAlways: buttons.get('approvalBtnAlways'),
approvalBtnDeny: buttons.get('approvalBtnDeny'),
msg,
}};
function $(id) {{ return elements[id] || null; }}
function syncTopbar() {{}}
function hideApprovalCard() {{ card.classList.remove('visible'); }}
function _clearApprovalHideTimer() {{}}
function _syncApprovalCollapseButton() {{}}
function _syncApprovalTranscriptSpace() {{}}
function applyLocaleToDOM() {{}}
function setTimeout(fn) {{ return 1; }}
function showToast() {{}}
function setStatus() {{}}
function t(key) {{ return key; }}
function api() {{
apiCalls += 1;
return new Promise(resolve => {{ resolveApi = resolve; }});
}}
{helpers}
const pending = {{
approval_id: 'appr-1',
_session_id: 'sess-1',
command: 'rm -rf /tmp/test',
description: 'Dangerous command approval',
pattern_key: 'dangerous_command',
pattern_keys: ['dangerous_command'],
}};
_approvalPendingBySession.set('sess-1', {{ pending, pendingCount: 1 }});
showApprovalCard(pending, 1);
const first = respondApproval('once');
_renderPendingApprovalForActiveSession();
const second = respondApproval('once');
output.apiCallsDuringFlight = apiCalls;
output.onceDisabledDuringFlight = buttons.get('approvalBtnOnce').disabled;
output.onceLoadingDuringFlight = buttons.get('approvalBtnOnce').classList.contains('loading');
resolveApi({{ ok: false }});
Promise.all([first, second]).then(() => {{
output.apiCallsFinal = apiCalls;
output.onceDisabledAfter = buttons.get('approvalBtnOnce').disabled;
output.onceLoadingAfter = buttons.get('approvalBtnOnce').classList.contains('loading');
output.cardVisibleAfter = card.classList.contains('visible');
process.stdout.write(JSON.stringify(output));
}});
"""
out = _run_node(script)
assert out["apiCallsDuringFlight"] == 1
assert out["onceDisabledDuringFlight"] is True
assert out["onceLoadingDuringFlight"] is True
assert out["apiCallsFinal"] == 1
assert out["onceDisabledAfter"] is False
assert out["onceLoadingAfter"] is False
assert out["cardVisibleAfter"] is True