From 6baf501dcef3a01bf422a0713a8879c4f53fcf4c Mon Sep 17 00:00:00 2001 From: nesquena-hermes Date: Sat, 4 Jul 2026 08:41:47 +0000 Subject: [PATCH] =?UTF-8?q?harden=20#5471:=20dual-signature=20suppression?= =?UTF-8?q?=20(Opus=20root=20fix)=20=E2=80=94=20covers=20stale=20prefix=20?= =?UTF-8?q?+=20attachment?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Opus found the exact-match signature fails open in the COMMON case: the server's last persisted draft is often a PREFIX of the submitted text (Enter-to-send within 400ms cancels the pending debounced save), so matching only the submitted text misses it and the prefix restores (the literal 'tail of the already-submitted message' symptom). Root fix (Opus-recommended): capture BOTH signatures at send — the remembered SERVER-draft signature (_composerDraftPayloadSignatureForSid, read before the reset) AND the submitted-payload signature — and suppress a restore matching EITHER. The prefix IS the remembered server draft; the remembered files went through the same lossy serialization so they self-match; a genuinely new cross-tab draft matches neither and restores. Subsumes the file-canonicalization fix (kept as belt-and-suspenders). Suppression stores signatures[] not signature. Regression tests: stale-prefix suppressed (Opus Finding 1), file round-trip self-matches (Codex Finding 2), cross-tab-different still restores. Test harness updated to model send-time server draft vs later restore draft as distinct. Co-authored-by: ruizanthony --- static/sessions.js | 35 +++++++++++------ .../test_issue5060_boot_composer_preserve.py | 39 ++++++++++++++++++- 2 files changed, 61 insertions(+), 13 deletions(-) diff --git a/static/sessions.js b/static/sessions.js index f688fba5..5cd329e3 100644 --- a/static/sessions.js +++ b/static/sessions.js @@ -86,17 +86,28 @@ function _composerDraftPayloadSignatureForSid(sid) { function _suppressComposerDraftRestoreAfterSubmit(sid, text, files) { if (!sid) return; const previous = _composerDraftRestoreSuppressedUntilBySid.get(sid); - let signature = null; + // Collect EVERY signature a stale poll could legitimately echo back for this + // just-sent turn, and suppress a restore matching ANY of them (#5471): + // - the submitted-payload signature (final textarea content on send), AND + // - the REMEMBERED SERVER DRAFT signature — what was actually persisted last. + // The two differ in the common Enter-to-send case: `_clearComposerDraft` + // cancels the pending debounced save, so the server's last draft is often a + // PREFIX of the submitted text (pause ≥400ms, then send within 400ms of the + // last keystroke) — an exact submitted-text match would miss that prefix and + // let it restore. The remembered-server-draft signature must be read BEFORE + // the `_rememberComposerDraftPayloadState(sid,'',[])` reset below. A genuinely + // new cross-tab draft matches neither, so it still restores immediately. + const signatures = []; + const _addSig = (s) => { if (s && signatures.indexOf(s) === -1) signatures.push(s); }; + _addSig(_composerDraftPayloadSignatureForSid(sid)); // remembered server draft (read first) if (arguments.length >= 2) { - signature = _composerDraftPayloadSignature(text, files); - } else if (previous && typeof previous === 'object') { - signature = previous.signature || null; - } else { - signature = _composerDraftPayloadSignatureForSid(sid); + _addSig(_composerDraftPayloadSignature(text, files)); // submitted payload + } else if (previous && typeof previous === 'object' && Array.isArray(previous.signatures)) { + previous.signatures.forEach(_addSig); } _composerDraftRestoreSuppressedUntilBySid.set( sid, - { until: Date.now() + _COMPOSER_DRAFT_RESTORE_SUPPRESS_MS, signature }, + { until: Date.now() + _COMPOSER_DRAFT_RESTORE_SUPPRESS_MS, signatures }, ); // Local state must reflect the submitted/cleared composer immediately. The // POST that clears the server-side draft is async; same-session refreshes can @@ -119,11 +130,13 @@ function _isComposerDraftRestoreSuppressed(sid, text, files) { _composerDraftRestoreSuppressedUntilBySid.delete(sid); return false; } - const signature = (suppression && typeof suppression === 'object') ? suppression.signature : null; + const signatures = (suppression && typeof suppression === 'object' && Array.isArray(suppression.signatures)) + ? suppression.signatures + : null; // Legacy/unknown callers still fail closed for the current TTL, but all send - // paths now pass a payload signature so a different cross-tab draft can restore. - if (!signature) return true; - if (_composerDraftPayloadSignature(text, files) === signature) return true; + // paths now pass payload signatures so a different cross-tab draft can restore. + if (!signatures || !signatures.length) return true; + if (signatures.indexOf(_composerDraftPayloadSignature(text, files)) !== -1) return true; _composerDraftRestoreSuppressedUntilBySid.delete(sid); return false; } diff --git a/tests/test_issue5060_boot_composer_preserve.py b/tests/test_issue5060_boot_composer_preserve.py index e5dde40f..36807c6e 100644 --- a/tests/test_issue5060_boot_composer_preserve.py +++ b/tests/test_issue5060_boot_composer_preserve.py @@ -44,7 +44,7 @@ def _draft_restore_suppression_block() -> str: return SESSIONS_JS[start:end] -def _run_case(*, initial_text: str, draft: dict | None, opts: dict | None, current_sid, force_reload: bool, suppress_restore: bool | dict = False) -> dict: +def _run_case(*, initial_text: str, draft: dict | None, opts: dict | None, current_sid, force_reload: bool, suppress_restore: bool | dict = False, remembered_server_draft: dict | None = None) -> dict: if NODE is None: pytest.skip("node not on PATH") restore_fn = _function_block(SESSIONS_JS, "function _restoreComposerDraft(draft, targetSid, opts={}) {") @@ -60,6 +60,12 @@ def _run_case(*, initial_text: str, draft: dict | None, opts: dict | None, curre suppress_call = "_suppressComposerDraftRestoreAfterSubmit(sid);" else: suppress_call = "" + # At SEND time the session's composer_draft holds what was last persisted to + # the server (the prefix/old draft), which the dual-signature suppression + # reads. That is DISTINCT from the draft a later stale/cross-tab poll echoes + # back for restore. Seed the send-time remembered draft separately, then swap + # in the restore draft before _restoreComposerDraft runs. + send_time_draft = remembered_server_draft if remembered_server_draft is not None else draft script = f""" const state = {{ value: {json.dumps(initial_text)}, @@ -80,7 +86,7 @@ const sid = 'boot-session'; const S = {{ session: {{ session_id: sid, - composer_draft: {json.dumps(draft)}, + composer_draft: {json.dumps(send_time_draft)}, }}, }}; const currentSid = {json.dumps(current_sid)}; @@ -91,9 +97,15 @@ function _composerDraftHasPayload(text, files) {{ }} function _rememberComposerDraftPayloadState(sid, text, files) {{ state.rememberedDraft = {{sid, text, files}}; + if (S.session && S.session.session_id === sid) {{ + S.session.composer_draft = {{text, files}}; + }} }} {suppression_block} {suppress_call} +// After send-time suppression is captured, the stale/cross-tab poll delivers a +// (possibly different) draft that restore will evaluate. +S.session.composer_draft = {json.dumps(draft)}; {restore_fn} {draft_block} process.stdout.write(JSON.stringify({{ @@ -163,6 +175,7 @@ def test_same_session_submitted_clear_blocks_stale_server_draft_restore(): opts={"preserveActiveInput": True}, current_sid="boot-session", force_reload=True, + remembered_server_draft={"text": "old submitted suffix", "files": []}, suppress_restore={"text": "old submitted suffix", "files": []}, ) assert data["value"] == "" @@ -170,6 +183,27 @@ def test_same_session_submitted_clear_blocks_stale_server_draft_restore(): assert data["updateSendBtnCount"] == 0 +def test_same_session_submitted_clear_blocks_stale_prefix_server_draft_restore(): + """#5471 (Opus Finding 1): the server's last persisted draft is often a PREFIX + of the submitted text (Enter-to-send within 400ms of a keystroke cancels the + pending debounced save), so an exact-match on the submitted text would miss it. + The dual signature also captures the remembered SERVER draft, so the stale + prefix is suppressed. + """ + data = _run_case( + initial_text="", + draft={"text": "hello worl", "files": []}, # stale prefix echoed back + opts={"preserveActiveInput": True}, + current_sid="boot-session", + force_reload=True, + remembered_server_draft={"text": "hello worl", "files": []}, # server had the prefix + suppress_restore={"text": "hello world!", "files": []}, # user sent the full text + ) + assert data["value"] == "", "stale prefix of the just-sent text must be suppressed" + assert data["autoResizeCount"] == 0 + assert data["updateSendBtnCount"] == 0 + + def test_same_session_submitted_clear_allows_different_cross_tab_draft_restore(): """A different same-session draft saved by another tab must not be hidden by send-time suppression.""" data = _run_case( @@ -178,6 +212,7 @@ def test_same_session_submitted_clear_allows_different_cross_tab_draft_restore() opts={"preserveActiveInput": True}, current_sid="boot-session", force_reload=True, + remembered_server_draft={"text": "old submitted suffix", "files": []}, suppress_restore={"text": "old submitted suffix", "files": []}, ) assert data["value"] == "new cross-tab draft"