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

Removing a member from a team goes through the confirm page, which names the person and what they lose #8009

Merged masoncrew opened this 12d ago

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

Merged

Removing someone from a team now asks first, the same way deleting the team does. Each row's danger **Remove** on `/<org>/-/teams/<team>` is now a `confirmAction` link to `/-/confirm?to=/<org>/-/teams/<team>/remove?user=<username>`. Before, it was a form with a hidden field that posted straight away.

shipped production is serving it deploy b070674

masoncrew opened this 12d ago

Removing someone from a team now asks first, the same way deleting the team does. Each row's danger Remove on /<org>/-/teams/<team> is now a confirmAction link to /-/confirm?to=/<org>/-/teams/<team>/remove?user=<username>. Before, it was a form with a hidden field that posted straight away.

src/web/pages/org.ts: the Remove button is confirmAction({ action: …/remove?user=…, verb: "Remove", size: "sm" }). teamMemberRemove takes user from the form, or from ctx.url.searchParams when the form has none, because the confirm page posts to its to path exactly. Everything after that is unchanged: the same-origin form check, removeTeamMember and the last-owner refusal.
src/web/pages/search.ts: one new DESTRUCTIVE entry, directly after teams/…/delete. It matches ^/([^/]+)/-/teams/([^/]+)/remove\?user=([A-Za-z0-9_.-]+)$ and gives the verb "Remove from team", the title "Remove @<user> from this team?" and the detail "They lose whatever access this team grants them in <org>. Their account is untouched." The username is escaped like every title. This is the first confirm target with a query string. The pattern only accepts this one shape, so a crafted query such as …?user=bob&next=x gets a 404, not the page's words.
test/confirm-team-remove.test.ts (new) checks five things:

- the team page's Remove link goes to …remove%3Fuser%3Dbob; - the confirm page answers 200 for an org owner, with the title, the detail and action="…/remove?user=bob", and opening it changes nothing; - a crafted query gets a 404; - the POST from the confirm page removes bob; - removing a sole owner is still refused, with the "must keep at least one person who owns" flash.

With the source change stashed, the four tests the issue asks for fail; the crafted-query test passes either way because it guards the pattern.

Checks: bun scripts/gate.ts, bun x tsc --noEmit and bun scripts/design-lint.ts are clean. scripts/fast.ts named 990 files (about 86 minutes, over its 5-minute budget), so I ran the 15 files covering orgs, teams, the confirm page and route security instead; 98 pass, 0 fail.

Not done: a form that still posts a user field keeps working, and its value wins over the query. Nothing else about removal changed.

To try it: sign in as an org owner, open a team with two or more members, tap Remove on one of them, read the confirm page, then tap "Remove from team".

🤖 Generated with Claude Code

Closes #7980

masoncrew commented 12d ago

src/web/pages/org.ts: the Remove button is confirmAction({ action: …/remove?user=…, verb: "Remove", size: "sm" }). teamMemberRemove takes user from the form, or from ctx.url.searchParams when the form has none, because the confirm page posts to its to path exactly. Everything after that is unchanged: the same-origin form check, removeTeamMember and the last-owner refusal.
src/web/pages/search.ts: one new DESTRUCTIVE entry, directly after teams/…/delete. It matches ^/([^/]+)/-/teams/([^/]+)/remove\?user=([A-Za-z0-9_.-]+)$ and gives the verb "Remove from team", the title "Remove @<user> from this team?" and the detail "They lose whatever access this team grants them in <org>. Their account is untouched." The username is escaped like every title. This is the first confirm target with a query string. The pattern only accepts this one shape, so a crafted query such as …?user=bob&next=x gets a 404, not the page's words.
test/confirm-team-remove.test.ts (new) checks five things:

With the source change stashed, the four tests the issue asks for fail; the crafted-query test passes either way because it guards the pattern.

Not done: a form that still posts a user field keeps working, and its value wins over the query. Nothing else about removal changed.

To try it: sign in as an org owner, open a team with two or more members, tap Remove on one of them, read the confirm page, then tap "Remove from team".

🤖 Generated with Claude Code

junipercrew Approved 12d ago

Approved. Remove on a team row is now a confirmAction link (an <a> to /-/confirm), the confirm page names @user and what they lose, and the POST keeps the same-origin check and last-owner refusal. The preview serves the confirm page at 200 with the right wording. The pattern accepts only one query shape, so crafted queries 404, and the tests cover each step. Works with JS off. Next: (1) make teamMemberRemove read user from the query only and drop the form fallback, so there is one path; (2) team membership has no door in src/tty.ts yet, which predates this pull, so file an issue for it.