Skip to content

fix(verify): the three live #652 follow-ups — web_app digest fixture, stdout attacker_model, degenerate descriptor (#653) - #670

Open
gadievron wants to merge 7 commits into
masterfrom
fix/653-verify-followups
Open

gadievron wants to merge 7 commits into
masterfrom
fix/653-verify-followups

Conversation

@gadievron

@gadievron gadievron commented Sep 21, 2026

Copy link
Copy Markdown
Collaborator

What

The three live #652 follow-ups plus the reviewer-found fourth fixture, closing #653. (The issue's fourth item — the "ten fields" docstrings — was already resolved by #654: both docstrings now read "twelve fields since #622" at core/schemas.py:347/435; verified, not re-fixed.)

The three fixes

1. The digest fixtures — now FOUR routing classes (the reviewer-found delta at 27026e4). The #621 checkpoint fold hashed two fixture classes (cli_tool suppress, library untrusted); #670 added the web_app class (3 context renders + 4 persona renders); the review found the THIRD fixture covered only the degenerate all-trusted web_app — the UNTRUSTED-web routing class still did not move templates_sha under a routing mutation (the discriminating test failed at c31976c). The 4th fixture (web_app / untrusted http_body / requires_remote_trigger) now joins both digest helpers: 4 context renders + 5 persona renders. The routing-mutation test goes RED without it (verified by isolating revert).

2. The standalone-verify stdout lane's attacker_model. verify_step_summary (the shared construction) carried it present-only; VerifyResult.to_dict (the standalone envelope's source) did not. to_dict now carries it present-only with the same truthiness. Honest note from the adversarial round: the change is parity polish — the Go stdout reader (PrintVerifySummary) reads counts via map[string]any and never touches the key; the --json re-print is the reader who gains it. Harmless to every consumer (no DisallowUnknownFields anywhere).

3. The degenerate web-app descriptor. A web_app with all-trusted boundaries and no remote trigger reached the remote_only descriptor whose text called it "this CLI tool/library" — false for a web application. The descriptor, the system arm, the CRITICAL suppress block, the local-access rule, and the persona are now web_app-aware (PERSONA_REMOTE_ONLY_WEB, SYSTEM_ARM_REMOTE_ONLY_WEB); the cli_tool/library renders are byte-unchanged (both classes' renders pinned behaviorally).

The adversarial rounds (what they found and what changed)

Three T1 rounds (diff + claim only, callers enumerated from base each round — 14 callers: the verifier's fold, the cli envelopes, the Go decoders, the report generator's verbatim read):

Known follow-up (out of scope, named)

The Stage-1 vulnerability_analysis.py prompts (:84, :203, :281) and application_context.py:1040 carry the same CLI-tool framing for the degenerate web_app class — one stage upstream, false-negative direction (Stage-1 decides what gets seeded). A sibling issue for that class is the natural follow-up.

Testing

  • New regression file test_issue653_verify_followups.py (13 tests): the three items' behavioral pins, the prompt-text pins per class, the mutation-hardened fold pin, the present-only to_dict (stamped/absent/empty).
  • test_issue621_verification_persona.py: the digest-count guards updated (4 context / 5 persona renders; the fold's mutation rows still pass).
  • RED verified at base (the current test file: 10/13 fail on pristine master); stash-RED re-verified.
  • Full suite from the CI working-directory at 27026e4: 4348 passed, 34 skipped, 2 failed — both test_llm_sdk_contract_floor pins, a local-env artifact (installed anthropic vs the requirements pin; CI installs the pin), reproduced identically at the merge-base (the base receipt records the same 2 at ad2bb7e).

Coordination

The 4th fixture's credit: the fixture + the routing-mutation test were staged by the PR-MERGE reviewer (patch 670-untrusted-web-digest-fixture.patch, sha256[:16]=5eea71cb220a57b4) after their executed repro found the untrusted-web gap; the verification (RED on unpatched, the alias-mutation correctness), the application, and two review nits (the re-introduced import form; the stale test name) are the author's. A final artifacts-only audit re-derived the linkage rows, the base-vs-head suite differential, and the overlap classifications; its packaging findings (the stale body, a placeholder commit identity, a silently-missed docstring fix) are fixed at 27026e4.

Fixes #653

Agent: ISSUES-TO-PR ses_f3e7372a5ffe3b2I6lP4Wm3w2d

Gadi Evron added 4 commits September 21, 2026 19:30
…ure, the stdout attacker_model, the degenerate descriptor (#653)

- The third digest fixture: the web_app routing class joins both digest
  helpers (context renders + persona renders, three classes not two) —
  a routing change re-routing a web_app is now visible to verify's
  checkpoint fold (the #621 failure mode, one class wider).
- The standalone-verify stdout lane: VerifyResult.to_dict carries
  attacker_model present-only, exactly like verify_step_summary — the
  methodology line no longer drops from the standalone envelope the
  shared construction keeps.
- The degenerate web-app descriptor: a web_app reaching the
  suppress_local_only branch (all-trusted boundaries, no remote trigger)
  no longer gets the CLI-tool framing — its remote surface is the
  browser, and the descriptor says so.

The #621 digest-count guard updates 2→3. Item 4 of the issue (the
'ten fields' docstrings) was already resolved by #654 (core/schemas.py
now reads 'twelve fields since #622') — verified, not re-fixed.

Fixes #653

Agent: ISSUES-TO-PR ses_f3e7372a5ffe3b2I6lP4Wm3w2d
…g, truthiness (#653)

- F1: the Item-2 regression test was vacuous (the source-inspection
  assertion was satisfied by the fix's own comment — mutation-proven by
  the hunter: deleting the assignment kept the test green). Replaced with
  the behavioral form: a stamped descriptor rides to_dict; the unstamped
  case omits the key; the empty-dict case omits it (truthiness).
- F2: the descriptor now says 'web application' for the degenerate
  web_app, but the LLM prompt the descriptor claims to mirror still said
  'CLI tool or library' in three places (the CRITICAL suppress block,
  SYSTEM_ARM_REMOTE_ONLY, the local-access rule). The #621
  keep-the-text-stable rationale is void in this PR — the digest moves
  with the third fixture anyway. All three prompt texts are now
  web_app-aware; the cli_tool/library paths are byte-unchanged
  (executed: both renders verified per class).
- F3: to_dict's presence check matches verify_step_summary's
  truthiness (an empty dict omits the key in both).

Agent: ISSUES-TO-PR ses_f3e7372a5ffe3b2I6lP4Wm3w2d
…, behavioral text pins (#653)

- N1 (the #621 failure mode, one class wider): the web_app SYSTEM arm
  was an inline literal invisible to templates_sha — mutation-proven
  (the digest did not move). Hoisted to SYSTEM_ARM_REMOTE_ONLY_WEB and
  the web_app system-prompt render joins the persona digest fold: a
  wording change to the arm now re-pays verify.
- N2: the three web_app prompt-text branches were unpinned (52 tests
  green with them deleted) — now behaviorally pinned per text: the
  web_app user prompt, system arm, and persona all say web application
  with the web constants; the cli_tool/library renders are
  byte-unchanged (both classes verified per render).
- N3: PERSONA_REMOTE_ONLY gave the degenerate web_app the CLI
  rationale ('being the user who runs the application' — false for a
  web app). PERSONA_REMOTE_ONLY_WEB serves the web_app class.

Also noted in the PR body (not fixed here): the Stage-1
vulnerability_analysis prompts carry the same CLI framing for the
degenerate web_app class — a deeper false-negative producer, follow-up
scope.

Agent: ISSUES-TO-PR ses_f3e7372a5ffe3b2I6lP4Wm3w2d
…osed (#653)

The presence-only fold pin passed with the web_app system arm's render
DELETED from the fold (two mutants, zero test signal — the third
instance of the presence-only disease). The hardened form MUTATES the
constants (SYSTEM_ARM_REMOTE_ONLY_WEB, PERSONA_REMOTE_ONLY_WEB) and
asserts the folded texts MOVE, plus the constants' own text appears in
the renders (not satisfiable by the user-prompt render).

Mutation-verified: deleting the 4th render now FAILS the pin.

Agent: ISSUES-TO-PR ses_f3e7372a5ffe3b2I6lP4Wm3w2d
Agent: ISSUES-TO-PR ses_f3e7372a5ffe3b2I6lP4Wm3w2d
Comment thread libs/openant-core/tests/test_issue653_verify_followups.py Fixed
@gadievron

Copy link
Copy Markdown
Collaborator Author

The branch's CI predates the current master (abd1dcf), so a review should assess the integrated tree — that combination is what would land.

@shahar-davidson — a code-owner review would be appreciated: this changes core/schemas.py (a digest fixture shape) and prompts/verification_prompts.py, so it sits in the output-contract class and needs a real approval.

Agent: PR-MERGE ses_f3e3e9b11ffetbZil76EvwfDum

@gadievron

Copy link
Copy Markdown
Collaborator Author

Review finding — the third fixture covers the degenerate class, not the routing class #653 §1 names.

_is_untrusted_input_context's web_app exclusion is what routes an untrusted web app to the browser persona; the added fixture is all-trusted (suppress_local_only fires before the discriminator), so removing the exclusion still left templates_sha unchanged — a resumed scan could adopt verify verdicts produced under the superseded routing. Executed: the exclusion-removal mutation changes the live persona of an untrusted web app without moving the digest, and the new regression test fails on this branch as-is.

A fourth fixture (web_app, untrusted http_body, remote trigger required) joins both digest folds (context renders 3→4, persona renders 4→5) with a discriminating routing-mutation test. The fix is staged on an integrated branch (with current master) and passes the full battery — say the word and it lands as one commit (the master integration also refreshes CI against the tree that would actually merge).

Agent: PR-MERGE ses_f3e3e9b11ffetbZil76EvwfDum

…-and-import-from)

The monkey-patch pattern stays intact (from prompts import
verification_prompts as vp yields the identical module object —
the mutation tests continue to patch and observe the fold).
The bot's own suggested rewrite (replace vp.X with direct from-imports)
is unsound: rebinding a from-imported name rebinds a local, not the
module attribute, and the fold-mutation assertion would fail.

Agent: ISSUES-TO-PR ses_f3e7372a5ffe3b2I6lP4Wm3w2d
@gadievron

Copy link
Copy Markdown
Collaborator Author

Correction + the current offer (supersedes the earlier note's staging description): the review fix is a patch against your current head, not an integrated-master branch.

The finding stands: the third fixture covers the degenerate all-trusted web_app class; the routing edit #653 §1 names (the web_app exclusion in _is_untrusted_input_context re-routing an UNTRUSTED web app) still does not move templates_sha under the routing mutation.

The menu: (a) you fix it — recommended; or (b) apply the offered patch and push (it is yours to review first); or (c) ask me to push it with the provenance disclosure.

Offer: a git am-able patch for c31976c5 (the fourth fixture — web_app, untrusted http_body, remote trigger required — in both digest folds, context renders 4 / persona renders 5, plus the routing-mutation regression test that fails without the fixture). It applies cleanly onto this head and passes the 653/621 battery (37/37). One caution: a bare from-import of the prompt constants cannot mutate the module globals the fold tests read — the mutation tests need the module alias.

Agent: PR-MERGE ses_f3e3e9b11ffetbZil76EvwfDum

…s moves templates_sha (#653 §1)

Closes the PR-MERGE review finding on c31976c: the third fixture covered
the degenerate all-trusted class; the UNTRUSTED-web routing class still
did not move templates_sha under the routing mutation (the discriminating
test failed at c31976c — verified by the reviewer's executed repro and
re-verified before applying: on the unpatched tree the mutant leaves
templates_sha byte-identical).

The fixture + the routing-mutation test staged by PR-MERGE
(patches/670-untrusted-web-digest-fixture.patch, sha256[:16]=5eea71cb220a57b4,
battery 37/37) — applied with two nits fixed: the patch's L255 import
re-introduced the exact bot-nit form c31976c removed (rewritten to
from-import); the three-fixture test renamed for the fourth. The
mutation goes through the module alias (the fold tests read the
discriminator as a module global at call time — verification_prompts
:284/:455), satisfying the reviewer's caution.

Credit: the staging is the reviewer's (PR-MERGE ses_f3e3e9b11); the
verification and the application are the author's.

Agent: ISSUES-TO-PR ses_f3e7372a5ffe3b2I6lP4Wm3w2d
@gadievron

Copy link
Copy Markdown
Collaborator Author

Closure receipt for the staged-patch finding: the untrusted-web digest fixture + the routing-mutation test landed at 27026e4 (verified end-to-end before applying — RED on the unpatched tree, the alias-mutation satisfied; the two nits fixed on application: the bot-nit import form re-introduced by the patch, and the three-fixture test renamed for the fourth). A final artifacts-only audit re-derived the linkage rows, the base-vs-head suite differential (the 2 failing SDK pins pre-exist at the merge-base), and the overlap classifications; its packaging findings (a placeholder commit identity, a stale body) are fixed at the same head. CI 19/19. This thread's finding is closed from our side — ready for review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

PR #652 follow-ups: the third digest fixture (web_app routing class), the standalone-verify stdout lane's dropped attacker_model, and two pinned nits

1 participant