opencode-bot: changes for issue #191 #200

opencode-bot · 2026-09-18T18:41:22Z

opencode-bot commented

2026-09-18 18:41:22 +00:00

Closes #191.

opencode-bot added 1 commit

2026-09-18 18:41:22 +00:00

Launch: check the target, and let the server write the cookie

CI / build (pull_request) Successful in 43s

Details

fe3d09e587

/launch now asks BoardService.CanViewAsync about any remembered path
ShareLink reads as board-scoped, the way the .ics endpoint does, and
falls back to the default board when it is gone or no longer this
member's — board.missing was the one dead end a member could open on.
A visitor who is not signed in passes through untouched.

The cookie is written by the server now (POST /launch/remember, 204
with Set-Cookie: HttpOnly, SameSite=Lax, 30 days) instead of by
document.cookie, which WebKit caps at seven days on every iOS browser.
rememberPage in prefs-interop.js is a fetch with keepalive so a
navigation that closes the page does not cancel it; the endpoint
refuses anything but Sec-Fetch-Site: same-origin, and the antiforgery
middleware 400s a cross-site POST ahead of it anyway.

prefs.spec.js pins the HttpOnly month and the launch onto a deleted
board; docs/ui.md and the changelog bullet follow.

opencode-bot referenced this pull request

2026-09-18 18:41:23 +00:00

Last page: check the target, and let the server write the cookie #191

opencode-bot commented

2026-09-18 18:41:24 +00:00

🤖 Reviewing this PR — the review will link the full session transcript.

🤖 Reviewing this PR — the review will link the full session transcript.

opencode-reviewer approved these changes

2026-09-18 18:43:55 +00:00

opencode-reviewer left a comment

✅ Verdict: approve — no blocking issues found.

Good to merge. The server-side cookie write is sound: LastPage.Remember still gates both directions (and the launch root in NotPlaces keeps /launch/remember from ever being remembered itself), ShareLink.TryParse with a null origin only accepts rooted relative paths so the launch-time board check can't be pointed off-site, and the unauthenticated pass-through preserves the sign-in-with-destination flow. The WebKit 7-day cap on document.cookie is real, and the cookie encoding round-trips consistently between the server write, the reader, and the e2e test's hand-set cookie. One comment inaccuracy worth fixing (below): the antiforgery middleware does not actually 400 this endpoint, so the Sec-Fetch-Site header check is the only gate, not a second one behind antiforgery. That doesn't block merge — the check does the work, and the cookie is harmless — but the comment claims a protection that isn't there, in three places (Program.cs, LastPage.cs, docs/ui.md).

⏱ 2m 31s · 7 steps · $0.0139 · tokens: 61.5k in / 5.5k out (+65.0k cache-read) · Go: 5h 27% · wk 35% · mo 20%
📜 Full session transcript

✅ **Verdict: approve** — no blocking issues found. Good to merge. The server-side cookie write is sound: `LastPage.Remember` still gates both directions (and the `launch` root in `NotPlaces` keeps `/launch/remember` from ever being remembered itself), `ShareLink.TryParse` with a null origin only accepts rooted relative paths so the launch-time board check can't be pointed off-site, and the unauthenticated pass-through preserves the sign-in-with-destination flow. The WebKit 7-day cap on `document.cookie` is real, and the cookie encoding round-trips consistently between the server write, the reader, and the e2e test's hand-set cookie. One comment inaccuracy worth fixing (below): the antiforgery middleware does not actually 400 this endpoint, so the `Sec-Fetch-Site` header check is the only gate, not a second one behind antiforgery. That doesn't block merge — the check does the work, and the cookie is harmless — but the comment claims a protection that isn't there, in three places (Program.cs, LastPage.cs, docs/ui.md). --- --- ⏱ 2m 31s · 7 steps · $0.0139 · tokens: 61.5k in / 5.5k out (+65.0k cache-read) · Go: 5h 27% · wk 35% · mo 20% 📜 [Full session transcript](https://git.grandiras.net/Grandiras/opencode-bot/src/branch/opencode-logs/transcripts/Grandiras/Ponente/pr-200/run-2026-09-18T18-41-24-082Z.md)

src/Ponente.Kanban/Program.cs

					
				@ -370,0 +403,4 @@

				// rides along on), and the same month.

				//

				// Same-origin only. The cookie is harmless, but nothing off-site has a reason

				// to set it, and this is a POST — the one header is the whole of the CSRF

This comment is factually wrong: the antiforgery middleware does not 400 a cross-site POST here. UseAntiforgery only validates endpoints carrying IAntiforgeryMetadata with RequiresValidation=true, which minimal APIs add automatically only to form-binding endpoints — a JSON-body endpoint like this one has no such metadata, and even for metadata-carrying endpoints the middleware records a verdict rather than short-circuiting. So the Sec-Fetch-Site check is the only gate, not "the second gate". The check itself is adequate (the cookie is harmless and a forged write buys an attacker nothing), but the comment should say the header check is the whole of the CSRF story, as the earlier paragraph already does. The same wrong claim appears in LastPage.cs's docs and in docs/ui.md.

This comment is factually wrong: the antiforgery middleware does **not** 400 a cross-site POST here. `UseAntiforgery` only validates endpoints carrying `IAntiforgeryMetadata` with `RequiresValidation=true`, which minimal APIs add automatically only to form-binding endpoints — a JSON-body endpoint like this one has no such metadata, and even for metadata-carrying endpoints the middleware records a verdict rather than short-circuiting. So the `Sec-Fetch-Site` check is the *only* gate, not "the second gate". The check itself is adequate (the cookie is harmless and a forged write buys an attacker nothing), but the comment should say the header check is the whole of the CSRF story, as the earlier paragraph already does. The same wrong claim appears in `LastPage.cs`'s docs and in `docs/ui.md`.

opencode-bot commented

2026-09-18 18:43:55 +00:00

🔧 Let opencode-bot fix this — tick the box and opencode-bot picks up the review above, on this PR branch. Nothing runs until you do.

@opencode-bot Address the findings of the latest review on this pull request.

🔧 **Let opencode-bot fix this** — tick the box and opencode-bot picks up the review above, on this PR branch. Nothing runs until you do. - [ ] @opencode-bot Address the findings of the latest review on this pull request.

Grandiras merged commit 612a08d6cc into main

2026-09-18 18:45:29 +00:00

Grandiras deleted branch opencode/issue-191

2026-09-18 18:45:30 +00:00

Grandiras referenced this pull request from a commit

2026-09-18 18:45:30 +00:00

opencode-bot: changes for issue #191 (#200)

Grandiras referenced this pull request

2026-09-22 21:19:18 +00:00

Member-owned boards and a home page: decisions and order of work #190

Sign in to join this conversation.

No reviewers