Reconcile indexer route display with submit ops on mismatch (#450 / SEC-I02 H09) #994

Merged
PlasticDigits merged 3 commits from qa/450-route-intermediate-reconcile-notify into main 2026-07-01 00:39:21 +00:00
PlasticDigits commented 2026-06-30 22:20:15 +00:00 (Migrated from gitlab.com)

Summary

Addresses GitLab #450 (SEC-I02 H09) per review feedback on !987: when indexer intermediate_tokens disagrees with router_operations, the Swap page now reconciles the displayed route to the ops-derived path and shows a user-visible warning (swap-route-intermediate-reconciled) instead of silently falling through to pool-only or client BFS.

  • Adds reconcileSwapRouteIntermediateTokens() in swapRouteDisplay.ts (keeps existing swapRouteIntermediateTokensAligned for tests).
  • SwapPage uses reconciled tokens for display/submit and sets indexerRouteIntermediateReconciled on the quote payload.
  • Documents invariant in docs/security-model.md and skills/AGENTS_FRONTEND_SWAP_ROUTE_DISPLAY.md.

Acceptance checklist

Criterion Verification Result
Frontend derives intermediate token sequence from router_operations after indexer response tokenPathFromSwapOperations + reconcileSwapRouteIntermediateTokens unit tests PASS
Derived sequence compared against displayed intermediate_tokens before submission swapRouteIntermediateTokensAligned + reconcile tests PASS
Mismatch updates route and notifies user (no silent fall-through) SwapPage.test.tsx tampered intermediate_tokens case; data-testid="swap-route-intermediate-reconciled" PASS
Frontend test asserts tampered intermediate sequence is caught reconciles tampered indexer intermediate_tokens… test PASS

Third-party verification

# Unit tests
bash scripts/with-node.sh --cwd frontend-dapp -- npm run test:run -- \
  src/utils/swapRouteDisplay.test.ts src/pages/SwapPage.test.tsx

# Typecheck
bash scripts/with-node.sh --cwd frontend-dapp -- npx tsc --noEmit

Manual (optional): On / with a multihop CW20 quote, mock or tamper indexer intermediate_tokens so they disagree with router_operations → route row should show the ops path and the amber reconciliation warning under swap-route-summary; submit must not switch to client BFS (swap-route-source-client-fallback absent).

## Summary Addresses GitLab #450 (SEC-I02 H09) per review feedback on !987: when indexer `intermediate_tokens` disagrees with `router_operations`, the Swap page now **reconciles** the displayed route to the ops-derived path and shows a user-visible warning (`swap-route-intermediate-reconciled`) instead of silently falling through to pool-only or client BFS. - Adds `reconcileSwapRouteIntermediateTokens()` in `swapRouteDisplay.ts` (keeps existing `swapRouteIntermediateTokensAligned` for tests). - `SwapPage` uses reconciled tokens for display/submit and sets `indexerRouteIntermediateReconciled` on the quote payload. - Documents invariant in `docs/security-model.md` and `skills/AGENTS_FRONTEND_SWAP_ROUTE_DISPLAY.md`. ## Acceptance checklist | Criterion | Verification | Result | |-----------|--------------|--------| | Frontend derives intermediate token sequence from `router_operations` after indexer response | `tokenPathFromSwapOperations` + `reconcileSwapRouteIntermediateTokens` unit tests | PASS | | Derived sequence compared against displayed `intermediate_tokens` before submission | `swapRouteIntermediateTokensAligned` + reconcile tests | PASS | | Mismatch updates route and notifies user (no silent fall-through) | `SwapPage.test.tsx` tampered `intermediate_tokens` case; `data-testid="swap-route-intermediate-reconciled"` | PASS | | Frontend test asserts tampered intermediate sequence is caught | `reconciles tampered indexer intermediate_tokens…` test | PASS | ## Third-party verification ```bash # Unit tests bash scripts/with-node.sh --cwd frontend-dapp -- npm run test:run -- \ src/utils/swapRouteDisplay.test.ts src/pages/SwapPage.test.tsx # Typecheck bash scripts/with-node.sh --cwd frontend-dapp -- npx tsc --noEmit ``` **Manual (optional):** On `/` with a multihop CW20 quote, mock or tamper indexer `intermediate_tokens` so they disagree with `router_operations` → route row should show the ops path and the amber reconciliation warning under `swap-route-summary`; submit must **not** switch to client BFS (`swap-route-source-client-fallback` absent).
PlasticDigits commented 2026-06-30 22:22:36 +00:00 (Migrated from gitlab.com)

Security review — MR !994

Commit reviewed: 673446b23cbf38d93fc24a44a9cba3850062330f
Scope: Frontend route reconciliation for SEC-I02 H09 (#450) — reconcileSwapRouteIntermediateTokens, SwapPage quote/display/submit alignment, warning UI (swap-route-intermediate-reconciled), docs/skills updates; incidental Rust formatting-only diffs (no logic changes).

Outcome: FINDINGS: 0 medium+

Method

  • Traced attacker-controlled indexer fields (intermediate_tokens, router_operations, hops) through getRouteSolve → swapOperationsFromIndexerResponse → reconcileSwapRouteIntermediateTokens → computeSwapRouteDisplay / swapMutation (indexerOperations).
  • Verified enrichSwapOperationsWithHopMinReturns only attaches per-hop min_return (token addresses unchanged).
  • Checked submit snapshot gating via useSubmitAlignedSimQuote (#356) and static warning copy (no XSS/log-injection surface).
  • Confirmed Trade market panel derives route from ops only (no intermediate_tokens path); out of this MR’s diff.
  • No prior security-review threads on this MR to re-validate.

Assessment

This change closes the display/submit mismatch class (benign intermediate_tokens vs malicious router_operations): on mismatch the UI reconciles to the ops-derived path, surfaces an amber warning, and continues to submit indexer ops (no silent pool-only/client-BFS fallback that could mask tampering). Residual indexer trust (aligned evil paths, quote inflation) is documented in docs/security-model.md and outside this diff’s threat delta.

Inline threads: none (no medium+ findings).

## Security review — MR !994 **Commit reviewed:** `673446b23cbf38d93fc24a44a9cba3850062330f` **Scope:** Frontend route reconciliation for SEC-I02 H09 (#450) — `reconcileSwapRouteIntermediateTokens`, SwapPage quote/display/submit alignment, warning UI (`swap-route-intermediate-reconciled`), docs/skills updates; incidental Rust formatting-only diffs (no logic changes). **Outcome:** `FINDINGS: 0` medium+ ### Method - Traced attacker-controlled indexer fields (`intermediate_tokens`, `router_operations`, `hops`) through `getRouteSolve` → `swapOperationsFromIndexerResponse` → `reconcileSwapRouteIntermediateTokens` → `computeSwapRouteDisplay` / `swapMutation` (`indexerOperations`). - Verified `enrichSwapOperationsWithHopMinReturns` only attaches per-hop `min_return` (token addresses unchanged). - Checked submit snapshot gating via `useSubmitAlignedSimQuote` (#356) and static warning copy (no XSS/log-injection surface). - Confirmed Trade market panel derives route from ops only (no `intermediate_tokens` path); out of this MR’s diff. - No prior security-review threads on this MR to re-validate. ### Assessment This change **closes** the display/submit mismatch class (benign `intermediate_tokens` vs malicious `router_operations`): on mismatch the UI reconciles to the ops-derived path, surfaces an amber warning, and continues to submit indexer ops (no silent pool-only/client-BFS fallback that could mask tampering). Residual indexer trust (aligned evil paths, quote inflation) is documented in `docs/security-model.md` and outside this diff’s threat delta. **Inline threads:** none (no medium+ findings).
PlasticDigits commented 2026-07-01 00:07:27 +00:00 (Migrated from gitlab.com)

Must minimize user cognitive overload

Must minimize user cognitive overload
PlasticDigits commented 2026-07-01 00:13:53 +00:00 (Migrated from gitlab.com)

Bugfix follow-up (bd10a987)

Review item Fix
Must minimize user cognitive overload Shortened reconciliation notice from technical amber warning (Route path updated: indexer display disagreed with submit operations…) to brief "Route adjusted." with subtle ink-subtle styling (same tier as wrap/unwrap hints), keeping role="status" and swap-route-intermediate-reconciled test id. Route row still shows the ops-derived path; submit unchanged.

Tests: swapRouteDisplay.test.ts + SwapPage.test.tsx — 44/44 pass.

Thread resolution: Top-level note is non-resolvable (individual_note); addressed in this comment.

Label: agent:fix_bugfix removed.

## Bugfix follow-up (`bd10a987`) | Review item | Fix | |-------------|-----| | **Must minimize user cognitive overload** | Shortened reconciliation notice from technical amber warning (`Route path updated: indexer display disagreed with submit operations…`) to brief **"Route adjusted."** with subtle `ink-subtle` styling (same tier as wrap/unwrap hints), keeping `role="status"` and `swap-route-intermediate-reconciled` test id. Route row still shows the ops-derived path; submit unchanged. | **Tests:** `swapRouteDisplay.test.ts` + `SwapPage.test.tsx` — 44/44 pass. **Thread resolution:** Top-level note is non-resolvable (`individual_note`); addressed in this comment. **Label:** `agent:fix_bugfix` removed.
PlasticDigits commented 2026-07-01 00:14:21 +00:00 (Migrated from gitlab.com)

added 1 commit

  • bd10a987 - Shorten route reconciliation notice to reduce cognitive load.

Compare with previous version

added 1 commit <ul><li>bd10a987 - Shorten route reconciliation notice to reduce cognitive load.</li></ul> [Compare with previous version](/PlasticDigits/cl8y-dex-terraclassic/-/merge_requests/276/diffs?diff_id=1879515793&start_sha=673446b23cbf38d93fc24a44a9cba3850062330f)
PlasticDigits commented 2026-07-01 00:16:25 +00:00 (Migrated from gitlab.com)

Security review — MR !994

Commit reviewed: bd10a987c81824fb023d1f2707f8cece9c5a67af
Scope: Frontend route reconciliation for SEC-I02 H09 (#450) — reconcileSwapRouteIntermediateTokens, SwapPage quote/display/submit alignment, reconciliation notice UI (swap-route-intermediate-reconciled), docs/skills updates; incidental Rust formatting-only diffs (no logic changes). Delta since prior review (673446b): commit bd10a987 shortens reconciliation copy/styling only.

Outcome: FINDINGS: 0 medium+

Method

  • Re-traced attacker-controlled indexer fields (intermediate_tokens, router_operations, hops) through getRouteSolve → swapOperationsFromIndexerResponse → reconcileSwapRouteIntermediateTokens → computeSwapRouteDisplay / swapMutation (indexerOperations).
  • Verified enrichSwapOperationsWithHopMinReturns only attaches per-hop min_return (token addresses unchanged between quote and submit).
  • Checked submit snapshot gating via useSubmitAlignedSimQuote (#356); static warning copy rendered as React text (no XSS/log-injection surface).
  • Re-validated post-bd10a987 notice change: route row still derives from reconciled ops path (primary control); subdued “Route adjusted.” copy does not restore display/submit divergence.
  • Confirmed Trade market panel unchanged (ops-only route); out of this MR’s diff.

Assessment

This change closes the display/submit mismatch class (benign intermediate_tokens vs malicious router_operations): on mismatch the UI reconciles to the ops-derived path and continues to submit indexer ops (no silent pool-only/client-BFS fallback that could mask tampering). Residual indexer trust (aligned malicious paths, quote inflation) is documented in docs/security-model.md and is outside this diff’s threat delta.

Inline threads: none (no medium+ findings).

## Security review — MR !994 **Commit reviewed:** `bd10a987c81824fb023d1f2707f8cece9c5a67af` **Scope:** Frontend route reconciliation for SEC-I02 H09 (#450) — `reconcileSwapRouteIntermediateTokens`, SwapPage quote/display/submit alignment, reconciliation notice UI (`swap-route-intermediate-reconciled`), docs/skills updates; incidental Rust formatting-only diffs (no logic changes). Delta since prior review (`673446b`): commit `bd10a987` shortens reconciliation copy/styling only. **Outcome:** `FINDINGS: 0` medium+ ### Method - Re-traced attacker-controlled indexer fields (`intermediate_tokens`, `router_operations`, `hops`) through `getRouteSolve` → `swapOperationsFromIndexerResponse` → `reconcileSwapRouteIntermediateTokens` → `computeSwapRouteDisplay` / `swapMutation` (`indexerOperations`). - Verified `enrichSwapOperationsWithHopMinReturns` only attaches per-hop `min_return` (token addresses unchanged between quote and submit). - Checked submit snapshot gating via `useSubmitAlignedSimQuote` (#356); static warning copy rendered as React text (no XSS/log-injection surface). - Re-validated post-`bd10a987` notice change: route row still derives from reconciled ops path (primary control); subdued “Route adjusted.” copy does not restore display/submit divergence. - Confirmed Trade market panel unchanged (ops-only route); out of this MR’s diff. ### Assessment This change **closes** the display/submit mismatch class (benign `intermediate_tokens` vs malicious `router_operations`): on mismatch the UI reconciles to the ops-derived path and continues to submit indexer ops (no silent pool-only/client-BFS fallback that could mask tampering). Residual indexer trust (aligned malicious paths, quote inflation) is documented in `docs/security-model.md` and is outside this diff’s threat delta. **Inline threads:** none (no medium+ findings).
PlasticDigits commented 2026-07-01 00:33:58 +00:00 (Migrated from gitlab.com)

added 12 commits

  • bd10a987...9b98249d - 11 commits from branch main
  • 1ce2e180 - Merge branch 'main' into qa/450-route-intermediate-reconcile-notify

Compare with previous version

added 12 commits <ul><li>bd10a987...9b98249d - 11 commits from branch <code>main</code></li><li>1ce2e180 - Merge branch &#39;main&#39; into qa/450-route-intermediate-reconcile-notify</li></ul> [Compare with previous version](/PlasticDigits/cl8y-dex-terraclassic/-/merge_requests/276/diffs?diff_id=1879535624&start_sha=bd10a987c81824fb023d1f2707f8cece9c5a67af)
PlasticDigits commented 2026-07-01 00:34:02 +00:00 (Migrated from gitlab.com)

Resolved merge conflicts with main (commit 1ce2e180).

Conflicts: pair/src/error.rs, router/src/error.rs, dex-common/src/blacklist.rs — all from #456 blacklist default-deny landing on main while this branch still had the pre-1.5.0 fail-open error text and dex-common unit tests.

Resolution: Kept main's BlacklistGuardUnavailable message and removed the stale dex-common test module (coverage is in blacklist_tests). #450 route-reconciliation frontend changes are unchanged.

Sanity: swapRouteDisplay.test.ts + SwapPage.test.tsx (44 tests) and tsc --noEmit pass.

Resolved merge conflicts with `main` (commit 1ce2e180). **Conflicts:** `pair/src/error.rs`, `router/src/error.rs`, `dex-common/src/blacklist.rs` — all from #456 blacklist default-deny landing on `main` while this branch still had the pre-1.5.0 fail-open error text and dex-common unit tests. **Resolution:** Kept `main`'s `BlacklistGuardUnavailable` message and removed the stale dex-common test module (coverage is in `blacklist_tests`). #450 route-reconciliation frontend changes are unchanged. **Sanity:** `swapRouteDisplay.test.ts` + `SwapPage.test.tsx` (44 tests) and `tsc --noEmit` pass.
PlasticDigits commented 2026-07-01 00:35:35 +00:00 (Migrated from gitlab.com)

Security review — MR !994

Commit reviewed: 1ce2e1800b78a8cff2daa44dc2d34281ff0e8e09
Scope: Frontend route reconciliation for SEC-I02 H09 (#450) — reconcileSwapRouteIntermediateTokens, SwapPage quote/display/submit alignment, reconciliation notice UI (swap-route-intermediate-reconciled), docs/skills updates. Delta since prior review (bd10a987): merge commit 1ce2e180 (main into feature branch); no additional security-relevant logic changes in the reconciliation path.

Outcome: FINDINGS: 0 medium+

Method

  • Re-traced attacker-controlled indexer fields (intermediate_tokens, router_operations, hops) through getRouteSolve → swapOperationsFromIndexerResponse → reconcileSwapRouteIntermediateTokens → computeSwapRouteDisplay / swapMutation (indexerOperations).
  • Verified enrichSwapOperationsWithHopMinReturns only attaches per-hop min_return (token addresses unchanged between quote and submit).
  • Checked submit snapshot gating via useSubmitAlignedSimQuote (#356); static warning copy rendered as React text (no XSS/log-injection surface).
  • Re-validated prior threads (673446b, bd10a987): reconciliation still forces display to ops-derived path on mismatch; subdued “Route adjusted.” copy does not restore display/submit divergence.
  • Confirmed Trade market panel unchanged (ops-only route display; no intermediate_tokens path); out of this MR’s diff.

Assessment

This change closes the display/submit mismatch class (benign intermediate_tokens vs malicious router_operations): on mismatch the UI reconciles to the ops-derived path, surfaces a notice, and continues to submit indexer ops (no silent pool-only/client-BFS fallback that could mask tampering). Residual indexer trust (aligned malicious paths, quote inflation) is documented in docs/security-model.md and is outside this diff’s threat delta.

Inline threads: none (no medium+ findings).

## Security review — MR !994 **Commit reviewed:** `1ce2e1800b78a8cff2daa44dc2d34281ff0e8e09` **Scope:** Frontend route reconciliation for SEC-I02 H09 (#450) — `reconcileSwapRouteIntermediateTokens`, SwapPage quote/display/submit alignment, reconciliation notice UI (`swap-route-intermediate-reconciled`), docs/skills updates. Delta since prior review (`bd10a987`): merge commit `1ce2e180` (main into feature branch); no additional security-relevant logic changes in the reconciliation path. **Outcome:** `FINDINGS: 0` medium+ ### Method - Re-traced attacker-controlled indexer fields (`intermediate_tokens`, `router_operations`, `hops`) through `getRouteSolve` → `swapOperationsFromIndexerResponse` → `reconcileSwapRouteIntermediateTokens` → `computeSwapRouteDisplay` / `swapMutation` (`indexerOperations`). - Verified `enrichSwapOperationsWithHopMinReturns` only attaches per-hop `min_return` (token addresses unchanged between quote and submit). - Checked submit snapshot gating via `useSubmitAlignedSimQuote` (#356); static warning copy rendered as React text (no XSS/log-injection surface). - Re-validated prior threads (`673446b`, `bd10a987`): reconciliation still forces display to ops-derived path on mismatch; subdued “Route adjusted.” copy does not restore display/submit divergence. - Confirmed Trade market panel unchanged (ops-only route display; no `intermediate_tokens` path); out of this MR’s diff. ### Assessment This change **closes** the display/submit mismatch class (benign `intermediate_tokens` vs malicious `router_operations`): on mismatch the UI reconciles to the ops-derived path, surfaces a notice, and continues to submit indexer ops (no silent pool-only/client-BFS fallback that could mask tampering). Residual indexer trust (aligned malicious paths, quote inflation) is documented in `docs/security-model.md` and is outside this diff’s threat delta. **Inline threads:** none (no medium+ findings).
PlasticDigits (Migrated from gitlab.com) merged commit f7732c6706 into main 2026-07-01 00:39:22 +00:00
PlasticDigits commented 2026-07-01 00:39:27 +00:00 (Migrated from gitlab.com)

mentioned in commit f7732c6706

mentioned in commit f7732c6706e09f43fac0da51727be373cc253ea1
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
code/cl8y-dex-terraclassic!994
No description provided.