Skip to content

Commit 7429d45

Browse files
Toby1009claude
andcommitted
Half the analysis panel was a form that could not be submitted
Seven of the fourteen analysers on offer take an argument. The page renders an input for each name in their `REQUIRES` and sends the values with the request; `_run_over_store` called `instance.run(ctx, address=..., rows=...)` and nothing else. So filling in the field the tool asked for and pressing the button returned "linked_holders needs `addresses`" --- about the addresses just typed into it. common_funder, cross_chain, linked_holders, mixer, peel_chain, route and taint were all unreachable from the UI. Only the names an analyser declares are forwarded. Passing the whole query would have fixed the visible bug and opened a different one: a URL could then set any keyword argument on any analyser, including ones it never advertised. There is a test for each half. Found by filling the form in a browser. No test caught it because every test calls the analysers directly, which is the shape of gap that only using the thing finds. Two more from the same session, both "the button appears to do nothing": The parameter form and the result render below the fold in a panel taller than the window, so pressing an analyser changed nothing a reader could see and put the only signal in the opposite corner of the screen. Both scroll into view now, and the first field takes focus. And the ask dialog ignored Escape and let Tab walk out into the page behind it --- so the only way out was finding `close` with a mouse, and anyone navigating by keyboard ended up operating a graph they had been told was covered by a modal. Escape closes, Tab cycles inside, focus returns to whatever opened it, and it is a `role="dialog"` with `aria-modal`. The zoom controls were named "minus" and "plus" to a screen reader. The landing page's "Eight views" is now `VIEWS.length`. (The asset checkboxes and analyser buttons read as unnamed in one accessibility tool's tree. They are not --- the checkboxes sit inside labels, and the buttons take their name from their own text. Checked the markup rather than the tool this time; the same tool led me to report nineteen phantom findings once before.) Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FXWQ25VgFeMuuMhGWjvdof
1 parent 36334e8 commit 7429d45

6 files changed

Lines changed: 209 additions & 14 deletions

File tree

src/chainscope/server/local.py

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -830,7 +830,17 @@ def analyze(self, query: dict[str, list[str]]) -> dict[str, Any]:
830830
chain = self._chain(_first(query, "chain") or "1") or ChainId.evm(1)
831831

832832
rows = self._transfers(address, chain)
833-
found = _run_over_store(name, rows, address, chain, _first(query, "subject") or address)
833+
# Everything else on the query is a candidate analyzer argument. Which
834+
# of them are actually passed is decided by the analyzer's own
835+
# `REQUIRES`, in `_run_over_store`.
836+
extra = {
837+
key: values[0]
838+
for key, values in query.items()
839+
if key not in {"name", "address", "chain", "subject"} and values
840+
}
841+
found = _run_over_store(
842+
name, rows, address, chain, _first(query, "subject") or address, extra
843+
)
834844
return {
835845
"analyzer": name,
836846
"address": address,
@@ -1349,7 +1359,12 @@ def _asset_provider(chain: ChainId) -> Any:
13491359

13501360

13511361
def _run_over_store(
1352-
name: str, rows: list[Any], address: str, chain: ChainId, subject: str
1362+
name: str,
1363+
rows: list[Any],
1364+
address: str,
1365+
chain: ChainId,
1366+
subject: str,
1367+
extra: dict[str, str] | None = None,
13531368
) -> Any:
13541369
"""Dispatch to an analyzer that works over transfers already in the store.
13551370
@@ -1400,6 +1415,7 @@ def _run_over_store(
14001415
# returning an empty result that reads as "nothing found".
14011416
from ..cli.commands.analyze import _discover as discover
14021417

1418+
extra = extra or {}
14031419
registered, _ = discover()
14041420
factory = registered.get(name)
14051421
if factory is None:
@@ -1408,7 +1424,21 @@ def _run_over_store(
14081424
try:
14091425
instance = factory()
14101426
ctx = Context(chain=chain, router=None, limits={"rows": rows}) # type: ignore[arg-type]
1411-
return instance.run(ctx, address=address, rows=rows)
1427+
# What the analyzer declared it needs, taken from the request.
1428+
#
1429+
# These were collected by the page --- it renders an input per
1430+
# `REQUIRES` and sends them --- and then dropped here, so every
1431+
# analyzer with a required argument answered "linked_holders needs
1432+
# `addresses`" about the addresses you had just typed into the field it
1433+
# asked for. Seven of the fourteen on offer: common_funder,
1434+
# cross_chain, linked_holders, mixer, peel_chain, route, taint. Found
1435+
# by filling the form in a browser and pressing the button.
1436+
#
1437+
# Only the declared names are forwarded. Passing the whole query would
1438+
# let a URL set keyword arguments the analyzer never advertised.
1439+
wanted = tuple(getattr(instance, "REQUIRES", ()) or ())
1440+
supplied = {name: extra[name] for name in wanted if name in extra}
1441+
return instance.run(ctx, address=address, rows=rows, **supplied)
14121442
except AttributeError as exc:
14131443
# `router=None`: this handler runs analyzers over rows already in the
14141444
# store and has no provider to give them. An analyzer that reaches for
Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
"""An analyzer's declared arguments must survive the trip from the form.
2+
3+
The page renders an input for each name in an analyzer's ``REQUIRES`` and sends
4+
them with the request. `_run_over_store` then called ``instance.run(ctx,
5+
address=..., rows=...)`` and nothing else, so the values were dropped between
6+
the request and the analyzer --- and the analyzer answered "linked_holders
7+
needs `addresses`" about the addresses somebody had just typed into the field
8+
it asked for.
9+
10+
Seven of the fourteen analyzers on offer take an argument, so half the panel
11+
was a form that could not be submitted. Found by filling it in a browser, not
12+
by a test: every existing test called the analyzers directly.
13+
14+
The second test is the one that matters longer term. Forwarding the *whole*
15+
query would have fixed the visible bug and opened a different hole --- a URL
16+
could then set any keyword argument on any analyzer, including ones it never
17+
advertised. Only declared names travel.
18+
"""
19+
20+
from __future__ import annotations
21+
22+
from typing import Any, ClassVar
23+
24+
import pytest
25+
26+
from chainscope.core.chainid import ChainId
27+
from chainscope.core.result import Result
28+
from chainscope.server import local
29+
30+
CHAIN = ChainId.evm(1)
31+
SEED = "0x" + "11" * 20
32+
33+
34+
class _Spy:
35+
"""Stands in for a registered analyzer and records what it was handed."""
36+
37+
name = "spy"
38+
REQUIRES: ClassVar[tuple[str, ...]] = ("addresses", "source")
39+
seen: ClassVar[dict[str, Any]] = {}
40+
41+
def run(self, ctx: Any, **kwargs: Any) -> Result:
42+
type(self).seen = dict(kwargs)
43+
return Result(analyzer="spy")
44+
45+
46+
@pytest.fixture
47+
def spying(monkeypatch: pytest.MonkeyPatch) -> type[_Spy]:
48+
_Spy.seen = {}
49+
monkeypatch.setattr(
50+
"chainscope.cli.commands.analyze._discover",
51+
lambda: ({"spy": _Spy}, []),
52+
)
53+
return _Spy
54+
55+
56+
def test_a_declared_argument_reaches_the_analyzer(spying: type[_Spy]) -> None:
57+
local._run_over_store(
58+
"spy", [], SEED, CHAIN, SEED, {"addresses": f"{SEED},0xabc", "source": "0xdef"}
59+
)
60+
assert spying.seen["addresses"] == f"{SEED},0xabc"
61+
assert spying.seen["source"] == "0xdef"
62+
63+
64+
def test_an_undeclared_argument_does_not(spying: type[_Spy]) -> None:
65+
"""A query parameter is not a way to set arbitrary keyword arguments."""
66+
local._run_over_store("spy", [], SEED, CHAIN, SEED, {"addresses": SEED, "provider": "evil"})
67+
assert "provider" not in spying.seen
68+
69+
70+
def test_nothing_supplied_is_still_a_clean_call(spying: type[_Spy]) -> None:
71+
"""The analyzer's own default and its own error message, not a TypeError."""
72+
local._run_over_store("spy", [], SEED, CHAIN, SEED, {})
73+
assert "addresses" not in spying.seen
74+
assert spying.seen["address"] == SEED

web/src/app/page.tsx

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,8 +59,10 @@ export default function Home() {
5959
<section className="band frame" id="views">
6060
<h2 className="section">What it shows you</h2>
6161
<p className="lede">
62-
Eight views. Each one listed with what it is for, and with what an
63-
answer from it does not settle.
62+
{/* Counted, not written down. A ninth view would otherwise leave the
63+
prose saying eight, and the number is the first thing read. */}
64+
{VIEWS.length} views. Each one listed with what it is for, and with
65+
what an answer from it does not settle.
6466
</p>
6567
<div className="grid">
6668
{VIEWS.map((view, i) => (

web/src/components/ask-dialog.tsx

Lines changed: 54 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,13 @@
1414
* server, against a fixed vocabulary, and refuses rather than guesses.
1515
*/
1616

17-
import { useState } from "react";
17+
import { useEffect, useRef, useState } from "react";
1818

1919
import { Spinner } from "@/components/spinner";
2020
import { api, type AskReply } from "@/lib/api";
2121

22+
const FOCUSABLE = "a[href], button:not([disabled]), input, select, textarea, [tabindex]";
23+
2224
type Props = {
2325
chain: string;
2426
onClose: () => void;
@@ -30,6 +32,48 @@ export function AskDialog({ chain, onClose, onRun }: Props) {
3032
const [plan, setPlan] = useState<AskReply | null>(null);
3133
const [error, setError] = useState("");
3234
const [reading, setReading] = useState(false);
35+
const sheet = useRef<HTMLDivElement | null>(null);
36+
37+
/**
38+
* Escape closes, Tab stays inside, and focus goes back where it was.
39+
*
40+
* None of these held. Escape did nothing, so the only way out was finding
41+
* the close button with a mouse; Tab walked straight out of the dialog into
42+
* the page behind it, which is still there and still reachable, so somebody
43+
* navigating by keyboard or screen reader ends up operating a graph they
44+
* were told was covered by a modal. Verified by pressing Escape with the
45+
* dialog open and reading back `document.activeElement`, which was `BODY`.
46+
*/
47+
useEffect(() => {
48+
const returnTo = document.activeElement as HTMLElement | null;
49+
function onKey(event: KeyboardEvent) {
50+
if (event.key === "Escape") {
51+
event.preventDefault();
52+
onClose();
53+
return;
54+
}
55+
if (event.key !== "Tab" || !sheet.current) return;
56+
const stops = [...sheet.current.querySelectorAll<HTMLElement>(FOCUSABLE)].filter(
57+
(el) => el.offsetParent !== null,
58+
);
59+
if (!stops.length) return;
60+
const first = stops[0];
61+
const last = stops[stops.length - 1];
62+
const at = document.activeElement;
63+
if (event.shiftKey && (at === first || !sheet.current.contains(at))) {
64+
event.preventDefault();
65+
last.focus();
66+
} else if (!event.shiftKey && at === last) {
67+
event.preventDefault();
68+
first.focus();
69+
}
70+
}
71+
document.addEventListener("keydown", onKey);
72+
return () => {
73+
document.removeEventListener("keydown", onKey);
74+
returnTo?.focus?.();
75+
};
76+
}, [onClose]);
3377

3478
async function read() {
3579
if (!question.trim()) return;
@@ -55,8 +99,15 @@ export function AskDialog({ chain, onClose, onRun }: Props) {
5599

56100
return (
57101
<div className="scrim" onClick={onClose} role="presentation">
58-
<div className="sheet" onClick={(event) => event.stopPropagation()}>
59-
<h3>Ask in plain language</h3>
102+
<div
103+
className="sheet"
104+
ref={sheet}
105+
onClick={(event) => event.stopPropagation()}
106+
role="dialog"
107+
aria-modal="true"
108+
aria-labelledby="ask-title"
109+
>
110+
<h3 id="ask-title">Ask in plain language</h3>
60111
<p className="note small">
61112
Nothing is sent anywhere. The question is read on your own machine,
62113
against a fixed vocabulary, and you are shown what it would run before

web/src/components/graph.tsx

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -437,11 +437,18 @@ export function Graph({
437437
<button onClick={fit} title="fit the whole case on screen" aria-label="fit the whole case on screen">
438438
439439
</button>
440-
<button onClick={() => setView((v) => ({ ...v, zoom: Math.max(0.15, v.zoom * 0.85) }))}>
440+
{/* "minus" is what a screen reader reads off a bare glyph. */}
441+
<button
442+
aria-label="zoom out"
443+
onClick={() => setView((v) => ({ ...v, zoom: Math.max(0.15, v.zoom * 0.85) }))}
444+
>
441445
442446
</button>
443447
<span className="mono">{Math.round(view.zoom * 100)}%</span>
444-
<button onClick={() => setView((v) => ({ ...v, zoom: Math.min(3, v.zoom * 1.15) }))}>
448+
<button
449+
aria-label="zoom in"
450+
onClick={() => setView((v) => ({ ...v, zoom: Math.min(3, v.zoom * 1.15) }))}
451+
>
445452
+
446453
</button>
447454
</div>

web/src/components/selected.tsx

Lines changed: 35 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
* "unlabelled" the second state gets.
1616
*/
1717

18-
import { useCallback, useEffect, useState } from "react";
18+
import { useCallback, useEffect, useRef, useState } from "react";
1919

2020
import { Spinner } from "@/components/spinner";
2121
import { api, boot, type ExpandReply, type ResolveReply } from "@/lib/api";
@@ -60,6 +60,9 @@ export function Selected({
6060
// list lived in a literal here; the server knows what is actually installed.
6161
const [registered, setRegistered] = useState<Registered[]>([]);
6262
const [result, setResult] = useState<string>("");
63+
const paramsBox = useRef<HTMLDivElement | null>(null);
64+
const firstParam = useRef<HTMLInputElement | null>(null);
65+
const resultBox = useRef<HTMLPreElement | null>(null);
6366
const [params, setParams] = useState<Record<string, string>>({});
6467
const [open, setOpen] = useState<string | null>(null);
6568
const writable = boot().writable;
@@ -70,6 +73,29 @@ export function Selected({
7073
.catch(() => setRegistered([]));
7174
}, [chain]);
7275

76+
/**
77+
* Bring what just appeared into view, and put the cursor in it.
78+
*
79+
* This panel is taller than the window, so both the parameter form and the
80+
* result render below the fold: pressing an analyser that needs an argument
81+
* produced a field nobody could see and a status line at the far corner of
82+
* the screen, which reads exactly like a button that does nothing. Found by
83+
* pressing `common_funder` and watching the page not change.
84+
*
85+
* `block: "nearest"` rather than `"center"`, so a result already on screen
86+
* does not jump.
87+
*/
88+
useEffect(() => {
89+
if (!open) return;
90+
paramsBox.current?.scrollIntoView({ block: "nearest", behavior: "smooth" });
91+
firstParam.current?.focus();
92+
}, [open]);
93+
94+
useEffect(() => {
95+
if (!result) return;
96+
resultBox.current?.scrollIntoView({ block: "nearest", behavior: "smooth" });
97+
}, [result]);
98+
7399
const run = useCallback(
74100
async (item: Registered) => {
75101
onWork?.({ on: true, label: item.name });
@@ -291,10 +317,11 @@ export function Selected({
291317
</div>
292318

293319
{open ? (
294-
<div className="params">
295-
{(registered.find((r) => r.name === open)?.needs ?? []).map((need) => (
320+
<div className="params" ref={paramsBox}>
321+
{(registered.find((r) => r.name === open)?.needs ?? []).map((need, i) => (
296322
<input
297323
key={need}
324+
ref={i === 0 ? firstParam : undefined}
298325
className="mono"
299326
placeholder={need}
300327
value={params[need] ?? ""}
@@ -327,7 +354,11 @@ export function Selected({
327354
the pattern.
328355
</p>
329356
) : null}
330-
{result ? <pre className="result">{result}</pre> : null}
357+
{result ? (
358+
<pre className="result" ref={resultBox} tabIndex={-1}>
359+
{result}
360+
</pre>
361+
) : null}
331362

332363
{!writable ? (
333364
<p className="note small">

0 commit comments

Comments
 (0)