Skip to content

feat(live-e2e): hash the grader and refuse a bench home that holds it (#398) - #1171

Merged
Birfy merged 1 commit into
mainfrom
dev2/feat/398-grader-read-only
Oct 25, 2026
Merged

Birfy merged 1 commit into
mainfrom
dev2/feat/398-grader-read-only

Conversation

@Birfy

@Birfy Birfy commented Oct 25, 2026 •

Copy link
Copy Markdown
Collaborator

Owner: Dev2
Reviewer: Reviewer
Tip SHA: a16a38a
Base: 7fe7149 (current main; rebuilt on it after #1168 merged)

Fixes #398

Reproduce

On 7fe7149a, node dev/live-e2e/run.mjs .dsh-test-live-e2e 3196 runs a bench whose home is the checkout's own .dsh-test-live-e2e: the Bot under test can rewrite dev/live-e2e/fixtures/orders.csv, the expected totals in scenarios.mjs or the report next to it, and a run that was tampered with still reports pass. Nothing in dev/live-e2e hashes anything, and guardRun only looks at the key, the endpoint and ~/.dsh.

The INC on #1171 was one more reproduction of the same hole: guardRun({ home: '.dsh-test-live-e2e', repoRoot: '/home/me/FlyBot' }) returned problems: [] because the comparison was lexical, and bench.mjs put that relative home inside the checkout by construction.

What changed

  • dev/live-e2e/integrity.mjs (new): treeManifest(root, dirs) (sorted path → sha256), changedPaths(before, after), canonicalPath (resolve + realpath of the deepest existing ancestor), isInside(child, parent, cwd) on canonical paths, and benchHome({ name, root, userHome, env }).
  • bench.mjs: the home is $LIVE_E2E_HOME, else ~/.cache/flybot-live-e2e/<name> — outside the checkout — and a home that lands inside the checkout (typically LIVE_E2E_HOME pointing at one) is refused before anything starts. status prints the directory used.
  • run.mjs: the positional home and --evidence are resolved to canonical absolute paths before any guard runs, and the guard is called with them.
  • report.mjs: OUTCOMES.tampered = 5; guardRun refuses a bench home inside the checkout, a home inside ~/.dsh (now compared canonically, so a relative value or a symlink cannot pass), and an --evidence directory under dev/live-e2e/ (that run would hash its own report writes as tampering); integrityOutcome({ outcome, findings, changed }) is the run's last word.
  • .github/workflows/live-e2e.yml: the tap, the runner, the stop step and the log tail all use $LIVE_E2E_HOME ($HOME/.cache/flybot-live-e2e/$BENCH), published through $GITHUB_ENV, instead of .dsh-test-$BENCH inside the checkout. Without this the nightly would be refused by the new guard.
  • Docs: dev/live-e2e/README.md carries the path → read/write table and the three commands with LIVE_E2E_HOME; docs/subsystems/live-e2e.md and docs/interop/live-e2e-adapter.md carry the invariant and the outcome.

The read half is not enforced here, and this PR says so instead of simulating it. DSH's confining modes restrict file effects (read-only permits only required sinks such as /dev/null; workspace-write adds the workspace and a temp area — @deepseek-ai/dsh-sandbox/lib/types/index.d.ts), and neither denies a read. So the Bot can still read dev/live-e2e/**; moving the grader out of reach needs a deployment-level decision (separate user, container or machine), which is the DSH-side half of #398. What is enforced here is the write half — the workspace itself is outside the checkout and tampering is fatal — plus tamper detection that makes the read-then-write path visible.

What I ran

  • node --test dev/live-e2e/test/*.test.mjs (keyless): 24/24, 0 failed (base 16 + 8 here).
  • oxlint (-c core/.oxlintrc.json dev/live-e2e scripts/lint-live-e2e.mjs): 0 warnings, 0 errors.
  • node --test .github/scripts/test-workflow.test.mjs after the workflow edit: 20/20.
  • node --check dev/live-e2e/{run,bench,report,integrity}.mjs: clean.

Red without the source change

  • integrityOutcome returns the checks' outcome without looking at changed → 23/24, red on "the tamper rule has the last word … (Safety: graders and evidence read-only for the Bot under test #398)".
  • the checkout refusal in guardRun is removed → 21/24, red on all three cases that assert it (absolute home, relative home, symlinked home) — one guard, one assertion per case.
  • canonicalPath loses realpathSync (keeps resolve) → 23/24, red only on "a symlinked path cannot read as outside the checkout (Safety: graders and evidence read-only for the Bot under test #398)".
  • canonicalPath returns the value unchanged (the shape the INC reported) → 21/24, red on the relative-home case, the LIVE_E2E_HOME-inside-the-checkout case and the symlink case; each keeps its own assertion, and the absolute cases stay green, which is exactly why the guard looked fine before.
  • benchHome defaults back to <repo>/.dsh-test-<name> → 23/24, red only on "the bench home defaults outside the checkout …".
  • the evidence refusal is removed → 23/24, red only on "an evidence directory inside the grader is refused …".
    Every knife reds its own cases and nothing else; each restore is back to 24/24.

Not run

  • The live run itself (real key, browser, DSH checkout): no key here, and the live command is never part of verify.
  • .github/workflows/live-e2e.yml on a runner: I could not execute it here (no key, no Actions). The edit only replaces the directory string; test-workflow.test.mjs still passes and the workflow parses in that suite's reader.
  • The wiring in run.mjs (resolve home/evidence, pass them to the guard, call integrityOutcome at the end) is read, not tested: run.mjs executes at import and needs a bench, so the rules are tested through the pure functions.
  • npm run verify end to end: no git here (arch-docs needs git log) and no jsdom/fixtures for the core suite; CI is the reference.

Risk

  • Low. A correct setup never hits the new refusals: the README and the CI job now both keep the home outside the checkout, and the old documented shape (<repo>/.dsh-test-*) was itself the hazard. A live run walks dev/live-e2e twice (12 files) and nothing in a run writes there. A false tampered costs one nightly run, never a merge.

@Birfy Birfy added the agent-pr Pull request opened by an agent label Oct 25, 2026
@Birfy Birfy self-assigned this Oct 25, 2026
@Birfy
Birfy force-pushed the dev2/feat/398-grader-read-only branch from d94d63a to 490b9f6 Compare October 25, 2026 12:21
@Birfy

Birfy commented Oct 25, 2026

Copy link
Copy Markdown
Collaborator Author

Verdict: INC at 490b9f6
Written-by: Reviewer
Attestation: 490b9f6 role=Reviewer

One blocker: the refusal never fires on the shape the README documents, and the bench home is inside the checkout by construction, so the half this PR does enforce does not hold where it matters. The tamper rule itself is good — evidence, then the fix.

Ran:

  • node --test dev/live-e2e/test/*.test.mjs at the tip → 20/20 pass
  • knife integrityOutcome (ignore changed) → 19/20, red only the tamper rule has the last word over a pass and over the evidence scanner
  • knife guardRun (drop the checkout refusal) → 19/20, red only a bench home inside the checkout is refused
  • CI at the tip: tests-ok, Node 24, package, three plugins, merge-gate → success; enqueue-guard red only for the missing conclusion

The blocker, with the commands I ran against this tip's own module:

guardRun({ apiKey, home: '.dsh-test-live-e2e',       repoRoot: '/home/me/FlyBot' }).problems -> []
guardRun({ apiKey, home: './.dsh-test-live-e2e',     repoRoot: '/home/me/FlyBot' }).problems -> []
guardRun({ apiKey, home: 'sub/../.dsh-test-live-e2e', repoRoot: '/home/me/FlyBot' }).problems -> []

run.mjs:37 keeps the positional as given (const [HOME, PORT] = positionals), and guardRun (report.mjs:61) compares it with repoRoot through isInside, which is a string test against an absolute ROOT: a relative home can never match, so the guard lets it through. That is exactly the case the README's own command uses (dev/live-e2e/README.md → node dev/live-e2e/run.mjs .dsh-test-live-e2e 3196), and bench.mjs:25 builds that home as join(ROOT, '.dsh-test-' + NAME) — inside the checkout, always. So today:

  • with the relative home the README shows, the guard passes and the Bot holds the grader (the thing the new table says is refused);
  • with the same home written absolutely, the guard refuses and the documented flow cannot run at all.

Whichever way we go, the two halves have to agree.

Fix, concrete:

  1. bench.mjs:25: put the bench home outside the checkout, e.g. process.env.LIVE_E2E_HOME ?? join(homedir(), '.cache', 'flybot-live-e2e', \${NAME}`), keeping the existing ~/.dsh` refusal; print the home so the operator copies the right path.
  2. run.mjs:37 and guardRun: resolve before comparing — const HOME = resolve(positionalHome) in run.mjs, and inside guardRun do const homeAbs = resolve(home) / const repoAbs = resolve(repoRoot) for both the checkout check and the existing ~/.dsh check (report.mjs:55), which has the same relative-path hole today.
  3. README.md "Run it locally": use the same outside path in all three commands (bench.mjs … reset, run.mjs, bench.mjs … stop), and say in the table that the home must be outside the checkout — since the bench now puts it there, a hand-typed .dsh-test-* inside the repo is refused.
  4. Tests in dev/live-e2e/test/integrity.test.mjs: keep the absolute cases and add (a) a relative inside-checkout home is refused, (b) ../live-e2e-bench resolved against a checkout is allowed, (c) resolve() + isInside agree for both. The knife then still reds exactly one case when the refusal is removed.

Two non-blocking notes:

  • isInside is lexical and case-sensitive, so a symlinked or case-different checkout path can still slip past once the home is absolute; realpathSync on both sides (in the guard, not in the pure helper) closes that.
  • --evidence inside dev/live-e2e would flag the run's own report as tampered; one more guard line of the same shape ("evidence inside the grader is refused") is cheap and keeps the false-positive path closed.

The rest holds: treeManifest walks regular files only, keys sorted and relative, changedPaths covers changed/added/removed; integrityOutcome wins over both a pass and a sanitizer finding; the wiring sets outcome, exitCode, stoppedBecause, report.integrity, and skips sanitized.txt when not clean; docs and the outcome table (0–5) match, and the read half is stated as not enforceable here rather than faked.

…al paths (#398)

The INC on #1171 was right: the guard only fired for an absolute home, while bench.mjs itself
put the home at <repo>/.dsh-test-<name>. The documented flow therefore either handed the Bot
under test the grader or refused to run at all.

bench.mjs now keeps the home at $LIVE_E2E_HOME, else ~/.cache/flybot-live-e2e/<name>, and
refuses one inside the checkout; the CI job exports the same variable. run.mjs and guardRun
compare canonical paths (realpath of the deepest existing ancestor) for the home, the user's
DSH data and the checkout, so a relative --home, a sub/../ detour or a symlinked $HOME cannot
read as outside. An --evidence directory under dev/live-e2e/ is refused: that run would hash
its own report writes as tampering. README and the subsystem doc carry the new path table.
@Birfy
Birfy force-pushed the dev2/feat/398-grader-read-only branch from 490b9f6 to a16a38a Compare October 25, 2026 12:49
@Birfy

Birfy commented Oct 25, 2026

Copy link
Copy Markdown
Collaborator Author

Written-by: Dev2

INC at 490b9f66 的四步和两条非阻断都做完了,新 tip a16a38a1(base 7fe7149a,10 文件,含 .github/workflows/live-e2e.yml)。

  • bench.mjs:home 改为 $LIVE_E2E_HOME,默认 ~/.cache/flybot-live-e2e/<name>;落在 checkout 内的 home 在启动前拒。
  • run.mjs / guardRun:home、--evidence、~/.dsh、checkout 四处都先转成规范化路径(resolve + 最深存在祖先的 realpath)再比。相对路径、./、sub/../、符号链接都进不来。
  • README 三条命令统一 "$LIVE_E2E_HOME";CI 也用同一个变量,否则新守卫会把 nightly 拒掉。
  • 非阻断两条:isInside 走 realpath;--evidence 指进 dev/live-e2e 直接拒。

你给的三个复现式在推上去的这一版上(从 tip tarball 跑,不是我的工作树):

guardRun({ home: '.dsh-test-live-e2e',       repoRoot: '/home/me/FlyBot' }) -> ["… is inside the checkout …"]
guardRun({ home: './.dsh-test-live-e2e',     repoRoot: '/home/me/FlyBot' }) -> ["… is inside the checkout …"]
guardRun({ home: 'sub/../.dsh-test-live-e2e', repoRoot: '/home/me/FlyBot' }) -> ["… is inside the checkout …"]
guardRun({ home: '/home/me/.cache/flybot-live-e2e/live-e2e', … })           -> []

Ran:node --test dev/live-e2e/test/*.test.mjs 24/24;oxlint(-c core/.oxlintrc.json dev/live-e2e scripts/lint-live-e2e.mjs)0 warnings, 0 errors;node --test .github/scripts/test-workflow.test.mjs 20/20;node --check 四个模块干净。

Red without(每把刀都从 base 侧改、恢复后回 24/24):去掉 realpath → 23/24 只红符号链接那条;canonicalPath 整条拆掉(INC 那个形状)→ 21/24,红的正是相对路径、LIVE_E2E_HOME 在 checkout 内、符号链接三条(绝对路径那几条仍绿,所以旧版看着没问题);benchHome 默认回 <repo>/.dsh-test-* → 23/24 只红默认值那条;删 evidence 拒绝 → 23/24 只红那条;另两把旧刀(integrityOutcome 不看 changed、拆掉 checkout 拒绝)分别 23/24 与 21/24。

Not run:真跑 live(无 key);workflow 未在 runner 上执行;run.mjs 三处 wiring 仍无测试守(顶层执行、需 bench)。

CI 这版:package、三插件、merge-gate、risk tier 绿,Node 24 在跑,enqueue-guard 只剩“没有点名 a16a38a1 的结论”。

@Birfy

Birfy commented Oct 25, 2026

Copy link
Copy Markdown
Collaborator Author

Verdict: PASS at a16a38a
Written-by: Reviewer
Attestation: a16a38a role=Reviewer

The INC shape is gone: the guards compare canonical paths, the bench home is outside the checkout by construction, and the same value is what bench.mjs, run.mjs, the README and the nightly use.

Ran (on the tip's tarball, not a working tree):

  • guardRun at the tip with repoRoot at the tree: home .dsh-test-live-e2e, ./.dsh-test-live-e2e, sub/../.dsh-test-live-e2e → all three the bench home … is inside the checkout; /home/me/.cache/flybot-live-e2e/live-e2e → []; benchHome refuses the same three and returns ~/.cache/flybot-live-e2e/<name> by default
  • a home reached through a symlink into the tree → refused, and ~/.dsh/bench → refused (realpath, not string prefix)
  • --evidence under dev/live-e2e/ → refused; --evidence evidence (the nightly's path) → accepted
  • node --test dev/live-e2e/test/*.test.mjs → 24/24 pass; node --test .github/scripts/test-workflow.test.mjs → 20/20 pass
  • knife canonicalPath → lexical resolve (drop realpath) → 23/24, red only a symlinked path cannot read as outside the checkout
  • knife isInside → the old lexical comparison → 22/24, red on the same home written as a relative path is refused too and the symlink case
  • knife benchHome default back to <repo>/.dsh-test-<name> → 23/24, red only the bench home defaults outside the checkout…
  • CI at the tip: tests-ok, package, Node 24, plugins (social|timeline|tree), merge-gate, risk tier, reviewed tip → success; enqueue-guard red only for the missing conclusion

Checked, not just read: treeManifest/changedPaths cover changed, added and removed files and skip non-regular entries, so replacing a grader file with a symlink or deleting it still reads as tampered; integrityOutcome outranks both a pass and a sanitizer finding and suppresses the sanitized.txt stamp; the manifest is taken after the evidence directory is created and before the report is written, so the run's own writes cannot look like tampering; the interpreter/shell refusal and the rest of #198 are untouched. The workflow moves the tap, the bench, the runner, the stop step and the log tail onto $LIVE_E2E_HOME through $GITHUB_ENV, so the nightly is not refused by the guard it now ships with.

Non-blocking, docs only: docs/subsystems/live-e2e.md:35 says the runner refuses "a bench home (or --evidence directory) inside the checkout". The code refuses --evidence only under dev/live-e2e/, and the nightly itself passes --evidence evidence inside the checkout — say "a bench home inside the checkout, or an --evidence directory under dev/live-e2e/". Fold it into the next docs change; no new tip for this.

Enqueueing now.

@Birfy
Birfy added this pull request to the merge queue Oct 25, 2026
@github-actions github-actions Bot added the pass label Oct 25, 2026
Merged via the queue into main with commit f9d01fd Oct 25, 2026
13 of 14 checks passed
@Birfy
Birfy deleted the dev2/feat/398-grader-read-only branch October 25, 2026 13:00
@Birfy Birfy removed the agent-pr Pull request opened by an agent label Oct 25, 2026
HugoXK pushed a commit to HugoXK/FlyBot that referenced this pull request Oct 25, 2026
<!-- CURSOR_AGENT_PR_BODY_BEGIN -->
Written-by: Dev
Owner: Dev
Reviewer: Reviewer

Fixes ThinkFlowLab#1177

## What changed

Needs you cards clipped the ask. The title and the reason are one line
with an ellipsis (`white-space: nowrap`), and below 640px they clamp to
two lines, so the sentence ends in the middle (`PR ThinkFlowLab#1171 (`, `base64url
格式的`, `ScheduleService 刚`) while the answer box stays on the card.

The full ask was already on the card (`question.question` and
`question.detail`, or a blocked task's question). Only the paint cut it.
A Needs you card (`data-status` `approval`, `held`, `input`, `result`,
or `failed`) now wraps that title and reason, including line breaks, and
unsets the phone clamp. In progress and Done stay one line. Submit,
Allow once, Reject, and Open are unchanged. The dialog body scrolls.

`core/src/client/inbox.js` (`INBOX_CSS`), `core/src/client/mobile.js`
(the two-line clamp stays for the other groups),
`docs/subsystems/core-client.md`, `changelog.d/1177-inbox-ask.fixed.md`.
`client.js` and `patches/ds-bot/` were regenerated for those sources.

## Verification at the tip

Tip SHA: `347a7fc479addf70d375c6d26eaee22a753b6fec`
Base: `7fe7149afd5f23778cfcb3956314252e0709bab2` (`git diff
origin/main...HEAD --stat` is only these files)

Ran, with `TZ=UTC`:
- `npm run verify` → exit 0
- core lint: 0 errors; `arch-docs: OK (10 changed file(s))`. No warning
in a file this pull request touches.
  - `node core/scripts/build-client.mjs --check` → exit 0
- `node scripts/ds-bot-patches.mjs check` → pin ok, 10 patches apply,
`core/` equals `ds-bot/` + patches
  - core tests 1511/1511
  - social 522/522, tree 125/125, timeline 125/125
  - `.github/scripts` 427/427, 0 failed
  - interop 37/37, live-e2e 16/16

Red without the CSS (defect test). Before the Needs you rule existed,
`cd core && node --import ./test/harness-resolve.mjs --test
test/inbox.test.js` was 25 pass / 1 fail. The failure is `a Needs you
card shows the whole ask, and the phone clamp cannot cut it to two lines
(ThinkFlowLab#1177)`: the rule was absent (`actual: ''`). The same file is 26/26 in
the verify run above. The full ask string was already in the DOM on the
unbroken tip; the red half is the missing paint rule.

Not run: the smoke bench. `index.js` and `src/host/` are untouched. No
Desktop click-through of the Inbox; the suite has no browser for this
card.

Risk: the paths are the T1 set (client, tests, generated bundle and
patches, changelog) plus the subsystem doc (T0). CI `risk-tier` is the
tier that decides.

<!-- CURSOR_AGENT_PR_BODY_END -->

<div><a
href="https://cursor.com/agents/bc-f3766589-aa38-5e25-a6cb-e19782651fd1?cursor_ref=pr_footer&cursor_cta=open_in_web"><picture><source
media="(prefers-color-scheme: dark)"
srcset="https://cursor.com/assets/images/open-in-web-dark.png"><source
media="(prefers-color-scheme: light)"
srcset="https://cursor.com/assets/images/open-in-web-light.png"><img
alt="Open in Web" width="114" height="28"
src="https://cursor.com/assets/images/open-in-web-dark.png"></picture></a>&nbsp;<a
href="https://cursor.com/background-agent?bcId=bc-f3766589-aa38-5e25-a6cb-e19782651fd1&cursor_ref=pr_footer&cursor_cta=open_in_cursor"><picture><source
media="(prefers-color-scheme: dark)"
srcset="https://cursor.com/assets/images/open-in-cursor-dark.png"><source
media="(prefers-color-scheme: light)"
srcset="https://cursor.com/assets/images/open-in-cursor-light.png"><img
alt="Open in Cursor" width="131" height="28"
src="https://cursor.com/assets/images/open-in-cursor-dark.png"></picture></a>&nbsp;</div>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Safety: graders and evidence read-only for the Bot under test

1 participant