CLI sign-in: never preselect Admin, and warn when a device code came from elsewhere (#188)

* CLI sign-in: never preselect admin, and warn when a device code came from elsewhere
* Merge remote-tracking branch 'origin/main' into claude/cli-install-login-mcypgt
* CLI sign-in: treat only a two-letter code as the requester's country
* CLI sign-in: fall back to CF-IPCountry for the country, and give real-git import tests room

test-bot authored 10 days ago1 parent 52c2c20commit 64fcdfd18f9ec43c5e3f33eff550c1cdf10c44c8Browse files

CI / Typecheck and bundlesuccess
CI / Unit tests (1/2)success
CI / Unit tests (2/2)success
Deploy / Deploy to Cloudflaresuccess
End-to-end / End-to-end (agent reviews, tasks, Today)success
End-to-end / End-to-end (git, PRs, Actions)Failed
End-to-end / End-to-end (large pushes)success
End-to-end / End-to-end (passkeys, registry, email)success
End-to-end / End-to-end (real histories)Failed
End-to-end / End-to-end (security)success
Live test / Containers, registry, sandboxes, webhooks, schedules, LFS and pull requestssuccess

5 files changed+74−7

Files (5)

apps/diffinery/docs/accounts/cli-sign-in.md+3−3
apps/diffinery/src/http/cli-login.ts+1−1
apps/diffinery/src/web/cli-login.tsx+28−1
apps/diffinery/test/unit/cli-login.test.ts+39−0
containers/runner/test/import.test.ts+3−2

modifiedapps/diffinery/docs/accounts/cli-sign-in.md+3−3View file

modifiedapps/diffinery/src/http/cli-login.ts+1−1View file

modifiedapps/diffinery/src/web/cli-login.tsx+28−1View file

modifiedapps/diffinery/test/unit/cli-login.test.ts+39−0View file

modifiedcontainers/runner/test/import.test.ts+3−2View file

		@@ -56,7 +56,7 @@
56	56
57	57	Click Cancel instead and the CLI stops with `error: the sign-in was cancelled in the browser`.
58	58
59		-To ask for admin access up front (for `diffinery secret set`, for example), run `diffinery login --scope admin`. A command that needs more access than you signed in with says which `diffinery login --scope ...` to run.
	59	+To ask for admin access up front (for `diffinery secret set`, for example), run `diffinery login --scope admin`. The page still preselects Write and says the CLI asked for Admin: choose Admin yourself. A command that needs more access than you signed in with says which `diffinery login --scope ...` to run.
60	60
61	61	Then let Git use the same sign-in for `git clone`, `fetch` and `push` to diffinery.com:
62	62
		@@ -78,7 +78,7 @@
78	78	Waiting for you to approve (Ctrl+C to cancel)…
79	79	```
80	80
81		-Open `https://diffinery.com/login/device` on your laptop or phone, enter the code, check the details (the computer's name, and where and when the code was asked for), and click Authorize. The CLI finishes on its own within a few seconds. The code works for 10 minutes. The CLI uses this way by itself when it sees no display (no `DISPLAY` or `WAYLAND_DISPLAY` on Linux, or an SSH session); `--web` forces the browser way.
	81	+Open `https://diffinery.com/login/device` on your laptop or phone, enter the code, check the details (the computer's name, and where and when the code was asked for), and click Authorize. When the code was asked for from another country than the one your browser is in, the page says so in red; from another network, it notes that too. The CLI finishes on its own within a few seconds. The code works for 10 minutes. The CLI uses this way by itself when it sees no display (no `DISPLAY` or `WAYLAND_DISPLAY` on Linux, or an SSH session); `--web` forces the browser way.
82	82
83	83	### Scripts and CI
84	84
		@@ -103,7 +103,7 @@
103	103	## How it stays safe
104	104
105	105	- No secret in a link. The browser sign-in uses a one-time code that Diffinery sends only to the CLI on your own computer (`http://127.0.0.1:<port>`), and it works only together with a secret the CLI never lets out of its process (PKCE, as in OAuth for native apps). A sign-in link someone sends you can't sign their computer in as you: the code would go to your own computer.
106		-- Codes are typed, never linked. A device code is never part of a URL, so nobody can send you a link that approves their code. Only approve a code that your own terminal shows. If someone asks you to enter a code for them, don't.
	106	+- Codes are typed, never linked. A device code is never part of a URL, so nobody can send you a link that approves their code. Only approve a code that your own terminal shows. If someone asks you to enter a code for them, don't. The page warns when the code came from another country or network, and never preselects Admin, whatever the CLI asked for.
107	107	- Short-lived and single-use. Browser codes expire after 2 minutes and device codes after 10, each works once, and Diffinery stores only their hashes. The token is created when the CLI collects it and is never shown in the browser.
108	108	- The same rules as tokens. Approving needs a passkey sign-in from the last 2 hours, sign-ins are in the audit log (`token.create`), and a CLI token is limited by the access you chose like any token.
109	109

		@@ -27,7 +27,7 @@
27	27	const client = cleanClient((b.client ?? {}) as Record<string, unknown>);
28	28	const scope = requireScopeName(b.scope);
29	29	const cf = (c.req.raw as { cf?: { city?: string; country?: string } }).cf;
30		- const location = [cf?.city, cf?.country].filter(Boolean).join(", ") \|\| undefined;
	30	+ const location = [cf?.city, cf?.country ?? c.req.header("cf-ipcountry")].filter(Boolean).join(", ") \|\| undefined;
31	31	const d = await new CliLogins(c.env, c.get("dir")).startDevice(client, scope, { ip: c.req.header("cf-connecting-ip") ?? undefined, location });
32	32	const base = c.env.PUBLIC_URL.replace(/\/$/, "");
33	33	return c.json(

		@@ -61,9 +61,28 @@
61	61	</dl>
62	62	);
63	63
	64	+/**
	65	+ * Where a device code was asked from, compared with the browser approving it. Device-code phishing
	66	+ * means someone else's code: it was almost always asked for from another network, often another
	67	+ * country, so the page says so plainly.
	68	+ */
	69	+export function placeMismatch(from: { ip: string \| null; location: string \| null }, here: { ip?: string; country?: string }): "country" \| "network" \| null {
	70	+ // Stored as "City, CC" or "CC": the country is the two-letter code after the last comma. A city with no code is no country.
	71	+ const last = from.location?.split(",").at(-1)?.trim();
	72	+ const theirs = last && /^[A-Z]{2}$/.test(last) ? last : null;
	73	+ if (theirs && here.country && theirs !== here.country) return "country";
	74	+ if (from.ip && here.ip && from.ip !== here.ip) return "network";
	75	+ return null;
	76	+}
	77	+
	78	+/** A phisher's CLI can ask for any scope, so the page never preselects more than write: admin is always a deliberate pick. */
	79	+export const preselectedScope = (requested: TokenScope): TokenScope => (requested === "admin" ? "write" : requested);
	80	+
64	81	/** The approval form both flows share: the details, a scope, and Authorize or Cancel. */
65	82	const Approve: FC<{ c: AppContext; me: Account; action: string; client: CliClient; scope: TokenScope; hidden: Record<string, string>; from?: { ip: string \| null; location: string \| null; at: string }; code?: string }> = (p) => {
66	83	const t = csrfToken(p.c);
	84	+ const cf = (p.c.req.raw as { cf?: { country?: string } }).cf;
	85	+ const mismatch = p.from ? placeMismatch(p.from, { ip: p.c.req.header("cf-connecting-ip"), country: cf?.country ?? p.c.req.header("cf-ipcountry") }) : null;
67	86	return (
68	87	<Card title="Authorize the Diffinery CLI">
69	88	<p class="muted">
		@@ -75,13 +94,21 @@
75	94	</p>
76	95	) : null}
77	96	<ClientDetails client={p.client} from={p.from} />
	97	+ {mismatch === "country" ? (
	98	+ <div class="flash error" role="alert">
	99	+ <b>This code was asked for from another country</b> ({p.from!.location}) than the one you're in now. If you didn't run <code>diffinery login --device</code> there yourself, someone may be trying to get into your account: click Cancel.
	100	+ </div>
	101	+ ) : mismatch === "network" ? (
	102	+ <div class="flash warn">This code was asked for from a different network ({p.from!.ip}) than this browser's. That's expected when the computer and this browser are on different connections; if you didn't start it yourself, click Cancel.</div>
	103	+ ) : null}
78	104	<Form action={p.action} t={t}>
79	105	{Object.entries(p.hidden).map(([k, v]) => (
80	106	<input type="hidden" name={k} value={v} />
81	107	))}
82	108	<div class="field">
83	109	<label>Access</label>
84		- <KRadio name="scope" label="Access" value={p.scope} items={SCOPE_ITEMS} />
	110	+ <KRadio name="scope" label="Access" value={preselectedScope(p.scope)} items={SCOPE_ITEMS} />
	111	+ {p.scope === "admin" ? <div class="hint">The CLI asked for Admin. Choose it above only if you want this computer to change settings, secrets and variables.</div> : null}
85	112	</div>
86	113	<p class="muted small">
87	114	The CLI gets its own token, valid for {CLI_TOKEN_DAYS} days. It's listed under <a href="/settings/tokens">Settings → Access tokens</a>, where you can revoke it, and <code>diffinery logout</code> revokes it too.

		@@ -9,6 +9,7 @@
9	9	import { web } from "../../src/web/routes.js";
10	10	import type { Env } from "../../src/env.js";
11	11	import { cleanClient, formatUserCode, normalizeUserCode, pkceChallenge } from "../../src/auth/cli-login.js";
	12	+import { placeMismatch, preselectedScope } from "../../src/web/cli-login.js";
12	13	import { bucketsFor } from "../../src/lib/rate-limit.js";
13	14	import { testD1 } from "./stubs/d1.js";
14	15
		@@ -248,6 +249,44 @@
248	249	});
249	250	});
250	251
	252	+describe("guards against approving someone else's sign-in", () => {
	253	+ it("never preselects admin, and says when the CLI asked for it", async () => {
	254	+ const t = setup();
	255	+ const p = await pkce();
	256	+ const html = await (await t.browser().get(authorizeUrl(p, { scope: "admin" }))).text();
	257	+ expect(html).toContain(""value":"write"");
	258	+ expect(html).not.toContain(""value":"admin","items");
	259	+ expect(html).toContain("The CLI asked for Admin");
	260	+ expect(preselectedScope("admin")).toBe("write");
	261	+ expect(preselectedScope("read")).toBe("read");
	262	+ const plain = await (await t.browser().get(authorizeUrl(p))).text();
	263	+ expect(plain).not.toContain("The CLI asked for Admin");
	264	+ });
	265	+
	266	+ it("warns when the device code came from another network or country", async () => {
	267	+ const t = setup();
	268	+ const r = await t.fetch("/-/cli/device", { method: "POST", headers: { "content-type": "application/json", "cf-connecting-ip": "203.0.113.5", "cf-ipcountry": "NG" }, body: JSON.stringify({ client: { host: "far-away" } }) });
	269	+ const d = (await r.json()) as { user_code: string };
	270	+ const approve = (ip: string, country?: string) =>
	271	+ t.fetch("/login/device", {
	272	+ method: "POST",
	273	+ headers: { cookie: `__Host-dfy_session=ana-fresh-session-0000000000001; dfy_csrf=${CSRF}`, "content-type": "application/x-www-form-urlencoded", "content-length": String(new URLSearchParams({ _csrf: CSRF, code: d.user_code }).toString().length), origin: ORIGIN, "cf-connecting-ip": ip, ...(country ? { "cf-ipcountry": country } : {}) },
	274	+ body: new URLSearchParams({ _csrf: CSRF, code: d.user_code }).toString(),
	275	+ });
	276	+ expect(await (await approve("198.51.100.7")).text()).toContain("from a different network (203.0.113.5)");
	277	+ expect(await (await approve("203.0.113.5")).text()).not.toContain("different network");
	278	+ // The country comes from request.cf, or from the CF-IPCountry header when cf has none.
	279	+ expect(await (await approve("198.51.100.7", "TR")).text()).toContain("asked for from another country</b> (NG)");
	280	+ expect(await (await approve("198.51.100.7", "NG")).text()).not.toContain("another country");
	281	+ expect(placeMismatch({ ip: "1.1.1.1", location: "Lagos, NG" }, { ip: "2.2.2.2", country: "TR" })).toBe("country");
	282	+ expect(placeMismatch({ ip: "1.1.1.1", location: "TR" }, { ip: "2.2.2.2", country: "TR" })).toBe("network");
	283	+ expect(placeMismatch({ ip: "1.1.1.1", location: "Istanbul, TR" }, { ip: "1.1.1.1", country: "TR" })).toBeNull();
	284	+ expect(placeMismatch({ ip: null, location: null }, { ip: "1.1.1.1" })).toBeNull();
	285	+ expect(placeMismatch({ ip: "1.1.1.1", location: "Istanbul, TR" }, { ip: "2.2.2.2", country: "TR" })).toBe("network");
	286	+ expect(placeMismatch({ ip: "1.1.1.1", location: "Istanbul" }, { ip: "1.1.1.1", country: "TR" })).toBeNull();
	287	+ });
	288	+});
	289	+
251	290	describe("codes and client details", () => {
252	291	it("normalizes typed codes", () => {
253	292	expect(normalizeUserCode("bcdf-ghjk")).toBe("BCDFGHJK");

		@@ -70,6 +70,7 @@
70	70	const env = { ...process.env, GIT_AUTHOR_NAME: "T", GIT_AUTHOR_EMAIL: "t@example.com", GIT_COMMITTER_NAME: "T", GIT_COMMITTER_EMAIL: "t@example.com", GIT_CONFIG_GLOBAL: "/dev/null", GIT_CONFIG_NOSYSTEM: "1" };
71	71	const g = (cwd: string, ...args: string[]) => execFileSync("git", args, { cwd, env, stdio: "pipe" }).toString().trim();
72	72
	73	+ // Real git with many pushes: well past vitest's 5 s default on a busy runner.
73	74	it("pushes history in chunks under a size limit the server enforces, then every ref, and cleans up", async () => {
74	75	const root = mkdtempSync(path.join(tmpdir(), "diffinery-import-"));
75	76	const work = path.join(root, "work");
		@@ -147,7 +148,7 @@
147	148	const log = readFileSync(sizes, "utf8").trim().split("\n");
148	149	expect(log.every((l) => l.endsWith("diffinery-import"))).toBe(true);
149	150	expect(log.some((l) => Number(l.split(" ")[0]) > 153600)).toBe(true);
150		- });
	151	+ }, 30_000);
151	152	it("splits a refused batch of a large commit's files again, rather than retrying it whole", async () => {
152	153	const root = mkdtempSync(path.join(tmpdir(), "diffinery-import-"));
153	154	const work = path.join(root, "work");
		@@ -186,5 +187,5 @@
186	187	expect(result.ok).toBe(true);
187	188	expect(g(target, "rev-parse", "refs/heads/main")).toBe(g(work, "rev-parse", "HEAD"));
188	189	g(target, "fsck", "--no-progress");
189		- });
	190	+ }, 30_000);
190	191	});

codician/diffinerypublic

CLI sign-in: never preselect Admin, and warn when a device code came from elsewhere (#188)