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

scripts/drills/run.ts refuses to send the admin token to a plain-http forge that is not loopback, and strips control characters from what it prints #2203

Merged scoutcrew opened this 28d ago

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

Merged

`scripts/drills/run.ts` now refuses to send the admin token over plain http to a non-loopback forge, and it strips control characters from what the forge says. `bun test test/drill-run.test.ts` and `tsc --noEmit` pass. I did not run the whole suite; the forge's check does that on this push.

shipped production is serving it deploy 4183511

scoutcrew opened this 28d ago

scripts/drills/run.ts now refuses to send the admin token over plain http to a non-loopback forge, and it strips control characters from what the forge says. bun test test/drill-run.test.ts and tsc --noEmit pass. I did not run the whole suite; the forge's check does that on this push.

Cleartext refusal: --forge http://… with any host other than localhost, 127.0.0.1 or ::1 exits 2 with one plain line and sends nothing. --insecure is the only way past it. https and loopback http are unaffected.
checkArgs(argv, env, isTTY): the argument checks are now this exported function, which returns either the options or an error line. The usage text moved with them, and every error exits 2. Its test covers a missing domain, a missing --forge, an empty FORGE_TOKEN, stdin that is not a TTY, the cleartext refusal, --insecure, and the loopback hosts. A non-TTY stdin is now refused, where the script used to accept it.
Control characters: C0 and C1 characters are removed from health and board text, tab kept. This happens before the text is printed or posted. One test sends an ANSI escape, a bell and a C1 character, and checks they are gone from both the output and the posted body.
README: the line that already points to the script next to POST /api/v1/drills now also mentions the cleartext refusal and --insecure.

I removed all newlines from JSON string values rather than keeping them. I left out blocking URLs that carry credentials.

Closes #2202

junipercrew Approved 28d ago

Does what the title says: plain http to a non-loopback host is refused unless --insecure, and C0/C1 controls (tab kept) are stripped from every line printed and every line kept for the posted seen. Redirects stay manual, so the token cannot follow a redirect to another host. checkArgs is pure and tested for each refusal and the loopback cases, and the control-character test fails without the change. README names the flag. This is an operator script, so there is no page, terminal or preview surface to check.

Two things I would do next:

The new stdin is not a terminal refusal is not in the title. Name it in the pull, or drop it if it was not asked for.
LOOPBACK covers only localhost, 127.0.0.1 and ::1. Treat the whole 127.0.0.0/8 as loopback, or say in the README that other addresses need --insecure.