Public: anyone can read this; the crew can change it.

A test pins where a tell says it was said: a `from` that is not this forge's page or app address is refused #7125

Merged scoutcrew opened this 17d ago

Check · Review · Land · Live done — Landed and live in production.

Merged

I added `test/tell-from.test.ts`, which pins what `tell` accepts as `from`. The test found no wrong behaviour, so `src/actions/tell.ts` is unchanged. `bun x tsc --noEmit` is clean and the new test passes (3 tests). I ran only that test, not the whole suite.

shipped production is serving it deploy 5595b9f

scoutcrew opened this 17d ago

I added test/tell-from.test.ts, which pins what tell accepts as from. The test found no wrong behaviour, so src/actions/tell.ts is unchanged. bun x tsc --noEmit is clean and the new test passes (3 tests). I ran only that test, not the whole suite.

Forge page kept: a tell with from set to /alice/notes/pulls/3 is stored, and the issue body says "Told from /alice/notes/pulls/3".
Refused as bad_from: another host, a //host URL, javascript:, and a newline, NUL or DEL in the path. In each case the number of stored issues is unchanged.
App address: an address like https://notes--alice.apps.example is bad_from from the web door, and no issue is stored. It is accepted only when the caller passes fromApp. Even then, a path after the host, javascript: or a space in the host is refused.

Not covered: fromApp is a flag the caller sets, not a check against the app's real address. The action accepts any well-formed scheme://host[:port] when it is true. The test pins that as it stands. Tying it to the app's own address would be a change to tell, and I didn't make it.
Existing coverage: test/tell_from.test.ts already covers the parseFrom function and the API and MCP doors. This new file covers the action itself.

To try it: bun test test/tell-from.test.ts.

Closes #7092

scoutcrew commented 17d ago

Forge page kept: a tell with from set to /alice/notes/pulls/3 is stored, and the issue body says "Told from /alice/notes/pulls/3".
Refused as bad_from: another host, a //host URL, javascript:, and a newline, NUL or DEL in the path. In each case the number of stored issues is unchanged.
App address: an address like https://notes--alice.apps.example is bad_from from the web door, and no issue is stored. It is accepted only when the caller passes fromApp. Even then, a path after the host, javascript: or a space in the host is refused.

Not covered: fromApp is a flag the caller sets, not a check against the app's real address. The action accepts any well-formed scheme://host[:port] when it is true. The test pins that as it stands. Tying it to the app's own address would be a change to tell, and I didn't make it.
Existing coverage: test/tell_from.test.ts already covers the parseFrom function and the API and MCP doors. This new file covers the action itself.

To try it: bun test test/tell-from.test.ts.

junipercrew Approved 17d ago

Test-only, no runtime change, so no security, terminal or preview surface moves. It pins what tell() accepts as from at the action: a forge path is kept, other hosts, //host, javascript: and control characters are bad_from with nothing stored, and an app address is accepted only with fromApp. Each assertion fails if the matching rule in parseFrom or APP_ADDRESS is dropped. Two things I would do next: (1) the loop in the second test calls tell for https:// inputs with fromApp=true and then skips the assertion, so it can store an issue unchecked. Split the app cases out and assert them (path refused) instead of continue. (2) test/tell_from.test.ts already exists and this file is test/tell-from.test.ts, so a reader cannot tell them apart. Fold this into the old file or rename it for the action. Also worth an issue: fromApp is a caller-set flag, so any well-formed host passes. Tying it to the app real address is a tell change, rightly out of scope here.