Public: anyone can read this; the crew can change it.
Telling the crew from the toolbar carries the page you were on #4102
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/4091-telling-the-crew-from-the-toolbar-carrie → main 1 commit fork 1 thread · 1 unresolved
2 files
changed +42
−2
src/web/studio/toolbar.ts +4 −1
| @@ -127,7 +127,10 @@ export function toolbarPage(ctx: Ctx): Response { | ||
| 127 | 127 | () => |
| 128 | 128 | card( |
| 129 | 129 | html`<form method="post" action="${base}/tell" class="stack"> |
| 130 | <input type="hidden" name="from" value="${toolbarPath(base, where.env, "")}" /> | |
| 130 | ${address === "" | |
| 131 | ? html`<input type="hidden" name="from" value="${toolbarPath(base, where.env, "")}" />` | |
| 132 | : html`<input type="hidden" name="page" value="${address}" /> | |
| 133 | ${when(pull !== undefined, () => html`<input type="hidden" name="pull" value="${pull!.number}" />`)}`} | |
| 131 | 134 | <label>Tell the crew |
| 132 | 135 | <textarea name="text" rows="3" maxlength="${MAX_BODY}" required></textarea> |
| 133 | 136 | </label> |
test/toolbar-actions.test.ts +38 −1
| @@ -239,7 +239,7 @@ describe("toolbar actions", () => { | ||
| 239 | 239 | }); |
| 240 | 240 | |
| 241 | 241 | test("the page's tell form posts to the one tell door and the tell lands back on the page", async () => { |
| 242 | const text = await (await get(url("production"), aliceCookie)).text(); | |
| 242 | const text = await (await get("/alice/notes/toolbar?env=production", aliceCookie)).text(); | |
| 243 | 243 | const form = /<form method="post" action="([^"]+\/tell)"[^>]*>([\s\S]*?)<\/form>/.exec(text); |
| 244 | 244 | expect(form).not.toBeNull(); |
| 245 | 245 | const from = /name="from" value="([^"]+)"/.exec(form![2]!)![1]!.replaceAll("&", "&"); |
| @@ -255,6 +255,43 @@ describe("toolbar actions", () => { | ||
| 255 | 255 | expect(made[0]!.body).toContain("Told from"); |
| 256 | 256 | }); |
| 257 | 257 | |
| 258 | test("the tell form carries the page, and the pull on a preview, into the tell", async () => { | |
| 259 | const tellForm = async (env: string) => { | |
| 260 | const text = await (await get(url(env), aliceCookie)).text(); | |
| 261 | return /<form method="post" action="[^"]+\/tell"[^>]*>([\s\S]*?)<\/form>/.exec(text)![1]!; | |
| 262 | }; | |
| 263 | const prod = await tellForm("production"); | |
| 264 | expect(prod).toContain(`name="page" value="https://notes.example.test/a"`); | |
| 265 | expect(prod).not.toContain('name="pull"'); | |
| 266 | expect(prod).not.toContain('name="from"'); | |
| 267 | const res = await post("/alice/notes/tell", aliceCookie, { text: "Page tell prod", page: "https://notes.example.test/a" }); | |
| 268 | expect(res.status).toBe(200); | |
| 269 | const one = listIssues(app.db, repoId, {}).find((i) => i.title === "Page tell prod")!; | |
| 270 | expect(one.body).toContain("- page: https://notes.example.test/a"); | |
| 271 | expect(one.body).not.toContain("- pull:"); | |
| 272 | ||
| 273 | const prev = await tellForm(`pull-${pullNumber}`); | |
| 274 | expect(prev).toContain(`name="pull" value="${pullNumber}"`); | |
| 275 | await post("/alice/notes/tell", aliceCookie, { | |
| 276 | text: "Page tell preview", | |
| 277 | page: "https://notes.example.test/b", | |
| 278 | pull: String(pullNumber), | |
| 279 | }); | |
| 280 | const two = listIssues(app.db, repoId, {}).find((i) => i.title === "Page tell preview")!; | |
| 281 | expect(two.body).toContain("- page: https://notes.example.test/b"); | |
| 282 | expect(two.body).toContain(`- pull: #${pullNumber}`); | |
| 283 | }); | |
| 284 | ||
| 285 | test("an off-origin or over-long page is dropped from the form, and escaped", async () => { | |
| 286 | const bad = await (await get("/alice/notes/toolbar?env=production&page=javascript:alert(1)", aliceCookie)).text(); | |
| 287 | expect(bad).toContain('name="from"'); | |
| 288 | expect(bad).not.toContain("javascript:"); | |
| 289 | const long = await (await get(`/alice/notes/toolbar?env=production&page=${encodeURIComponent("https://a.test/" + "x".repeat(600))}`, aliceCookie)).text(); | |
| 290 | expect(long).toContain('name="from"'); | |
| 291 | const quote = await (await get(`/alice/notes/toolbar?env=production&page=${encodeURIComponent('https://a.test/"><script>')}`, aliceCookie)).text(); | |
| 292 | expect(quote).not.toContain("<script>"); | |
| 293 | }); | |
| 294 | ||
| 258 | 295 | test("a live run on a sealed issue is not shown to a reader who did not file it", async () => { |
| 259 | 296 | const sealed = createIssue(app.db, { |
| 260 | 297 | repoId, |
new above
offline
Next: validate address against the app's own origin here, not just http(s); today any origin in ?page= is carried into the tell.