Skip to content

Commit f5d0bff

Browse files
committed
fix(cloud): close P3.3 auth-boundary review findings
- BLOCKER: an unauthenticated request (no Authorization) no longer gets the shared companion token injected — the Worker only swaps in the shared token for an authenticated caller (shared token or valid invite), so the companion still gates /rpc (401) while /v1/health stays public. - Strip any client-supplied X-Helmor-Member-Id at the very top of fetch() (before proxyToSandbox) so the never-client-asserted invariant holds on every path, including the SDK preview path — not just the derived proxy hop. - acceptInvite: fail-closed on an unparseable expires_at, and reject re-accept of an already-claimed invite with a different id (409) to block seat takeover. Re-verified on real CF: no-auth->401, health->200, garbage->401, shared->200, accepted-invite->200, re-accept-different-id->409.
1 parent 3f51179 commit f5d0bff

2 files changed

Lines changed: 52 additions & 15 deletions

File tree

cloud/src/index.ts

Lines changed: 38 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -38,20 +38,29 @@ const sleep = (ms: number) => new Promise((r) => setTimeout(r, ms));
3838

3939
export default {
4040
async fetch(request: Request, env: Env): Promise<Response> {
41+
// Security (EVERY path): strip any client-supplied X-Helmor-Member-Id up
42+
// front — before proxyToSandbox — so the "never client-asserted" invariant
43+
// holds on the SDK preview path too, not only the derived proxy hop. The
44+
// companion trusts this header as author_id, so a client must never set it.
45+
const req = request.headers.has("X-Helmor-Member-Id")
46+
? new Request(request, { headers: withoutMemberHeader(request.headers) })
47+
: request;
48+
4149
// Preview-URL passthrough (port-subdomain hostnames). Returns null for
4250
// the Worker's own hostname, which we proxy transparently below.
43-
const proxied = await proxyToSandbox(request, env);
51+
const proxied = await proxyToSandbox(req, env);
4452
if (proxied) return proxied;
4553

4654
// Team registry (`/team/*`): D1-backed JSON routes. Returns null for any
4755
// other path, which falls through to the container proxy below.
48-
const url = new URL(request.url);
49-
const teamResp = await handleTeamRoute(request, env, url);
56+
const url = new URL(req.url);
57+
const teamResp = await handleTeamRoute(req, env, url);
5058
if (teamResp) return teamResp;
5159

52-
// Derive the member identity for the proxied hop. On a bad token this
53-
// short-circuits with 401 (we must not proxy an unknown caller).
54-
const forwarded = await deriveForwardedRequest(request, env);
60+
// Derive the member identity + companion-token swap for the proxied hop.
61+
// Unauthenticated calls pass through unswapped (the companion gates them:
62+
// /rpc -> 401, /v1/health stays public); unknown tokens -> 401 (see fn).
63+
const forwarded = await deriveForwardedRequest(req, env);
5564
if (forwarded instanceof Response) return forwarded;
5665

5766
const port = Number(env.HELMOR_COMPANION_PORT ?? "8080");
@@ -100,25 +109,36 @@ async function deriveForwardedRequest(
100109
): Promise<Request | Response> {
101110
const bearer = readBearer(request);
102111
let memberId: string | null = null;
103-
if (bearer && bearer !== env.HELMOR_COMPANION_TOKEN) {
112+
// Only swap in the shared companion token for an AUTHENTICATED caller — admin
113+
// (shared token) or a member (valid invite token). An UNauthenticated caller
114+
// must NOT receive the shared token (that would bypass the companion's own
115+
// bearer auth); pass it through unswapped so the companion gates it
116+
// (/rpc -> 401, /v1/health stays public). An unknown token -> 401 here.
117+
let injectCompanionToken = false;
118+
if (bearer === env.HELMOR_COMPANION_TOKEN) {
119+
injectCompanionToken = true; // admin / local (no member id)
120+
} else if (bearer) {
104121
memberId = await lookupMemberId(env, bearer);
105122
if (!memberId) {
106123
return new Response(JSON.stringify({ code: "Unauthorized" }), {
107124
status: 401,
108125
headers: { "content-type": "application/json" },
109126
});
110127
}
128+
injectCompanionToken = true; // member: invite token -> shared token
111129
}
112130

113131
const forwarded = new Request(request, {
114132
headers: new Headers(request.headers),
115133
});
116134
forwarded.headers.delete("X-Helmor-Member-Id"); // never client-asserted
117135
if (memberId) forwarded.headers.set("X-Helmor-Member-Id", memberId);
118-
forwarded.headers.set(
119-
"Authorization",
120-
`Bearer ${env.HELMOR_COMPANION_TOKEN}`,
121-
);
136+
if (injectCompanionToken) {
137+
forwarded.headers.set(
138+
"Authorization",
139+
`Bearer ${env.HELMOR_COMPANION_TOKEN}`,
140+
);
141+
}
122142
return forwarded;
123143
}
124144

@@ -130,6 +150,13 @@ function readBearer(request: Request): string | null {
130150
return match ? match[1] : null;
131151
}
132152

153+
/** Clone request headers with the trusted member-id header removed. */
154+
function withoutMemberHeader(headers: Headers): Headers {
155+
const next = new Headers(headers);
156+
next.delete("X-Helmor-Member-Id");
157+
return next;
158+
}
159+
133160
/** Ensure the companion server is up. Fast-path on a health hit; otherwise
134161
* launch the boot script and poll until it answers (Xvfb + serve cold start). */
135162
async function ensureServe(

cloud/src/team.ts

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -134,14 +134,24 @@ async function acceptInvite(request: Request, env: Env): Promise<Response> {
134134
}
135135

136136
const invite = await env.DB.prepare(
137-
"SELECT expires_at FROM invites WHERE token = ?1",
137+
"SELECT expires_at, member_id FROM invites WHERE token = ?1",
138138
)
139139
.bind(token)
140-
.first<{ expires_at: string | null }>();
140+
.first<{ expires_at: string | null; member_id: string | null }>();
141141
if (!invite)
142142
return json({ code: "NotFound", message: "unknown invite" }, 404);
143-
if (invite.expires_at && Date.parse(invite.expires_at) < Date.now()) {
144-
return json({ code: "Gone", message: "invite expired" }, 410);
143+
if (invite.expires_at) {
144+
// Fail-closed: an unparseable expires_at counts as expired (Date.parse ->
145+
// NaN, and `NaN < now` is false, which would otherwise never expire).
146+
const expiry = Date.parse(invite.expires_at);
147+
if (!Number.isFinite(expiry) || expiry < Date.now()) {
148+
return json({ code: "Gone", message: "invite expired" }, 410);
149+
}
150+
}
151+
// An already-claimed invite may be refreshed by the SAME member, but never
152+
// re-bound to a different id — no seat takeover via a leaked invite link.
153+
if (invite.member_id && invite.member_id !== githubId) {
154+
return json({ code: "Conflict", message: "invite already claimed" }, 409);
145155
}
146156

147157
const avatarUrl = typeof body.avatarUrl === "string" ? body.avatarUrl : null;

0 commit comments

Comments
 (0)