From 1be7baecc4c21fced947d76a824d7d19a57ca42c Mon Sep 17 00:00:00 2001 From: Jonathan Santilli <1774227+jonathansantilli@users.noreply.github.com> Date: Wed, 26 Aug 2026 10:25:10 +0100 Subject: [PATCH] fix(fleet): bound the token an enrolling server can make us write MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Investigating CodeQL's "network data written to file" alerts turned up one thing worth changing. The enrolment response's token is written straight into the user's home directory with no bound on its size, so a hostile or simply broken server can answer with a gigabyte of string and this persists all of it. A real token is `cgm_` plus 32 base64url bytes — under fifty characters. Capped at 1024, which leaves room for a longer token format without leaving room for that. The content feed already bounds its downloads the same way (8MB bundle, 4KB signature); this path had been missed. The other three alerts are examined in the pull request rather than changed. --- src/fleet/enrol-client.ts | 18 ++++++++++++++++++ tests/fleet/enrol-client.test.ts | 27 ++++++++++++++++++++++++++- 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/src/fleet/enrol-client.ts b/src/fleet/enrol-client.ts index 96b36cf..0d90daf 100644 --- a/src/fleet/enrol-client.ts +++ b/src/fleet/enrol-client.ts @@ -13,6 +13,17 @@ import { resolveMachineId, type MachineIdentityDeps } from "./machine-identity.j export const ENROL_PATH = "/api/agent/enrol"; const DEFAULT_TIMEOUT_MS = 30_000; + +/** + * The longest token this will accept and write to disk. + * + * A real one is `cgm_` plus 32 base64url bytes — under fifty characters. The + * bound exists because the value comes from whatever server the operator + * pointed at: without it, a hostile or broken one can return a gigabyte of + * string and this writes all of it into the user's home directory. Generous + * enough that a longer token format would still fit. + */ +const MAX_TOKEN_LENGTH = 1024; /** The token is a fleet credential; nobody else on the machine needs it. */ const FILE_MODE = 0o600; const DIR_MODE = 0o700; @@ -90,6 +101,13 @@ export async function enrolMachine( return { ok: false, reason: `${url} did not return a token` }; } + if (body.token.length > MAX_TOKEN_LENGTH) { + return { + ok: false, + reason: `${url} returned a token of ${body.token.length} characters, which is not a token this server should be issuing`, + }; + } + const configPath = fleetConfigPath(deps); const contents = `${JSON.stringify({ server, token: body.token }, null, 2)}\n`; diff --git a/tests/fleet/enrol-client.test.ts b/tests/fleet/enrol-client.test.ts index 9de2285..04f7cd2 100644 --- a/tests/fleet/enrol-client.test.ts +++ b/tests/fleet/enrol-client.test.ts @@ -1,4 +1,4 @@ -import { mkdtempSync, readFileSync, statSync } from "node:fs"; +import { existsSync, mkdtempSync, readFileSync, statSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { describe, expect, it } from "vitest"; @@ -201,6 +201,31 @@ describe("enrolMachine", () => { expect(result.reason).toContain("did not return a token"); }); + // The token is whatever the server sends, and this writes it into the + // user's home directory. A hostile or broken server returning an enormous + // string should be refused rather than persisted. + it("refuses a token far larger than any real one, instead of writing it", async () => { + const home = tempHome(); + const huge = `cgm_${"A".repeat(200_000)}`; + + const result = await enrolMachine( + { server: "https://g.example", code: "C" }, + { + homeDir: () => home, + fetch: (async () => + new Response(JSON.stringify({ token: huge }), { + status: 200, + headers: { "content-type": "application/json" }, + })) as unknown as typeof fetch, + }, + ); + + expect(result.ok).toBe(false); + if (result.ok) return; + expect(result.reason).toContain("not a token this server should be issuing"); + expect(existsSync(fleetConfigPath({ homeDir: () => home }))).toBe(false); + }); + it("says so when enrolment worked but the config could not be written", async () => { const result = await enrolMachine( { server: "https://g.example", code: "C" },