From c70d63abf9060354c9df768518ca9f3ba7e20327 Mon Sep 17 00:00:00 2001 From: tegwick Date: Wed, 16 Sep 2026 01:25:12 +0200 Subject: [PATCH] Handle pasted memo identifiers and explain invalid review input Assistant: codex Assistant-Model: gpt-6-astra Assistant-Session: 01a09cbb-87c6-7900-a145-4ce53ba9f1a6 --- Containerfile.memo-input | 3 +++ deploy/t03-admission.json | 2 +- docs/evidence/2026-09-16-memo-input-fix.json | 9 ++++++++ informed_decision/web.py | 13 +++++++---- tests/test_review_controller.py | 24 ++++++++++++++++++++ 5 files changed, 46 insertions(+), 5 deletions(-) create mode 100644 Containerfile.memo-input create mode 100644 docs/evidence/2026-09-16-memo-input-fix.json diff --git a/Containerfile.memo-input b/Containerfile.memo-input new file mode 100644 index 0000000..3adb3f8 --- /dev/null +++ b/Containerfile.memo-input @@ -0,0 +1,3 @@ +# Minimal repair on the admitted runtime; build with the repository as context. +FROM forgejo.coulomb.social/coulomb/informed-decision@sha256:8f55bcecf37a8d65f96e073510b1ffb4636c0a91d75e1ee7d582ad4bce8b953a +COPY --chown=10001:10001 informed_decision/web.py /opt/venv/lib/python3.12/site-packages/informed_decision/web.py diff --git a/deploy/t03-admission.json b/deploy/t03-admission.json index eb490a2..0ae98f0 100644 --- a/deploy/t03-admission.json +++ b/deploy/t03-admission.json @@ -1,5 +1,5 @@ { - "image": "forgejo.coulomb.social/coulomb/informed-decision@sha256:8f55bcecf37a8d65f96e073510b1ffb4636c0a91d75e1ee7d582ad4bce8b953a", + "image": "forgejo.coulomb.social/coulomb/informed-decision@sha256:ef6fdd61209863654be3adba070661645a2714e86cb4b7f4e847540291b55f07", "policy": { "origin": "http://flex-auth-informed-decision-t03.flex-auth.svc.cluster.local:8080", "package": "informed-decision.t03-review", diff --git a/docs/evidence/2026-09-16-memo-input-fix.json b/docs/evidence/2026-09-16-memo-input-fix.json new file mode 100644 index 0000000..4a65c7b --- /dev/null +++ b/docs/evidence/2026-09-16-memo-input-fix.json @@ -0,0 +1,9 @@ +{ + "task": "SECRETS-WP-0010-T03", + "reported_error": "Invalid request when opening apply memo", + "finding": "No version-2 policy observation or presentation reached the store. Exact ID passes the route regex; surrounding whitespace fails it. The original browser URL was requested but is not yet available, so whitespace is a reproduced failure mode, not a confirmed cause.", + "change": "Trim surrounding whitespace on GET memo lookup; explain invalid IDs; retain exact ID validation and duplicate/extra-parameter refusal.", + "validation": "119 passed: test_review_controller.py and test_browser_auth.py, INFD_APPROVAL_ENGINE_SOURCE=/home/worsch/approval-engine. Covers opening version 2, pasted whitespace, malformed and duplicate inputs, no approval entry during lookup.", + "image": "forgejo.coulomb.social/coulomb/informed-decision@sha256:ef6fdd61209863654be3adba070661645a2714e86cb4b7f4e847540291b55f07", + "human_approval_recorded": false +} diff --git a/informed_decision/web.py b/informed_decision/web.py index a063549..0b3d211 100644 --- a/informed_decision/web.py +++ b/informed_decision/web.py @@ -72,6 +72,7 @@ class App: messages = { "session_expired": "Your session expired. Sign in again to continue.", "invalid_form": "This form could not be verified. Reopen the review before submitting again.", + "invalid_memo_id": "Enter the memo identifier only, for example SECRETS-WP-0010-T03-apply. Do not include a URL or quotation marks.", "policy_denied": "The permission service refused this review action.", "wrong_recipient": "This review is addressed to another person.", "stale_presentation": "The memo changed. Open its current version before taking an action.", @@ -128,10 +129,14 @@ class App: if session is None: raise ReviewError(401, "session_expired") if method == "GET" and path == "/review": - params = _one(environ.get("QUERY_STRING", "")) - if set(params) != {"memo_id"} or not re.fullmatch(r"[A-Za-z0-9][A-Za-z0-9._:-]{0,255}", params["memo_id"]): - raise ValueError("invalid memo id") - page = self.review.open(session, params["memo_id"]) + try: + params = _one(environ.get("QUERY_STRING", "")) + except (ValueError, UnicodeError): + raise ReviewError(400, "invalid_memo_id") from None + memo_id = params.get("memo_id", "").strip() + if set(params) != {"memo_id"} or not re.fullmatch(r"[A-Za-z0-9][A-Za-z0-9._:-]{0,255}", memo_id): + raise ReviewError(400, "invalid_memo_id") + page = self.review.open(session, memo_id) return 200, review_page(page, session), "text/html", [] match = re.fullmatch(r"/presentations/(pres-[a-f0-9-]{36})(?:/(ack|act|packet/([0-9]{1,3})))?", path) if not match: diff --git a/tests/test_review_controller.py b/tests/test_review_controller.py index 629a64b..064e0cc 100644 --- a/tests/test_review_controller.py +++ b/tests/test_review_controller.py @@ -252,3 +252,27 @@ def test_renderer_release_cannot_silently_change_under_a_memo(review): controller.store.save_memo(memo.next_version(ui_release='informed-decision@0.1.0')) with pytest.raises(ReviewError,match='renderer_changed'):opened(review) assert controller.store.evidence()==[] + + +@pytest.mark.parametrize("padding", ["", " ", "\t\r\n", "\u00a0"]) +def test_pasted_memo_id_opens_current_version(review, padding): + controller, session, app, memo, engine, _ = review + controller.store.save_memo(memo.next_version()) + response = call(app, '/review', cookie=SESSION_COOKIE+'=fixture-session', + query=urlencode({'memo_id': padding+memo.id+padding})) + assert response['status'] == 200 + observations = controller.store.policy_observations() + assert json.loads(observations[-1]['request'])['context']['memo_version'] == 2 + assert not engine.get('fixture').entries + + +@pytest.mark.parametrize("query", ["memo_id=", "memo_id=+", "memo_id=%60memo-1%60", + "memo_id=memo-1&memo_id=memo-1", "memo_id=memo-1&extra=x", "memo_id=memo+1", + "memo_id=https%3A%2F%2Fdecisions.coulomb.social%2Freview", "memo_id=%FF"]) +def test_invalid_memo_input_explains_correction_before_policy(review, query): + controller, _, app, _, engine, _ = review + response = call(app, '/review', cookie=SESSION_COOKIE+'=fixture-session', query=query) + assert response['status'] == 400 + assert 'Enter the memo identifier only' in response['body'] + assert not controller.store.policy_observations() + assert not engine.get('fixture').entries