CLI: https only (plain http only for localhost), config saved privately, and say when the GitHub token is sent (#216)
* CLI: https only (plain http only for localhost), config saved privately, and say when the GitHub token is sent
6 files changed+57−6
Files (6)
modifiedapps/cli/src/commands/migrate.ts+8−0View file
| @@ -314,6 +314,12 @@ | ||
| 314 | 314 | } |
| 315 | 315 | |
| 316 | 316 | const ghToken = (ctx: Ctx) => ctx.io.env.GH_TOKEN || ctx.io.env.GITHUB_TOKEN || null; |
| 317 | + | |
| 318 | +/** Server imports send the ambient GitHub token along: say so, and to which server, rather than do it silently. */ | |
| 319 | +async function noteSendingToken(ctx: Ctx, token: string | undefined, what: string): Promise<void> { | |
| 320 | + if (!token) return; | |
| 321 | + ctx.err(ctx.c.yellow(`note: sending ${ctx.io.env.GH_TOKEN ? "GH_TOKEN" : "GITHUB_TOKEN"} to ${(await ctx.config()).url} so it can read ${what} from GitHub during the import`)); | |
| 322 | +} | |
| 317 | 323 | const ghApiUrl = (ctx: Ctx) => ctx.io.env.GITHUB_API_URL || undefined; |
| 318 | 324 | |
| 319 | 325 | /** |
| @@ -358,6 +364,7 @@ | ||
| 358 | 364 | github_token: ghToken(ctx) ?? undefined, |
| 359 | 365 | github_api_url: ghApiUrl(ctx), |
| 360 | 366 | }; |
| 367 | + await noteSendingToken(ctx, body.github_token, srcName); | |
| 361 | 368 | if (!ctx.json) ctx.out(`Starting server import of ${srcName} …`); |
| 362 | 369 | const created = await api.post<ImportJson>("/diffinery/imports", body); |
| 363 | 370 | const final = await pollImport(ctx, api, created.id); |
| @@ -491,6 +498,7 @@ | ||
| 491 | 498 | github_token: ghToken(ctx) ?? undefined, |
| 492 | 499 | github_api_url: ghApiUrl(ctx), |
| 493 | 500 | }; |
| 501 | + await noteSendingToken(ctx, body.github_token, org); | |
| 494 | 502 | if (!ctx.json) ctx.out(`Starting bulk import of ${org} into ${owner} …`); |
| 495 | 503 | const created = await api.post<ImportJson>("/diffinery/imports", body); |
| 496 | 504 | const final = await pollImport(ctx, api, created.id); |
modifiedapps/cli/src/config.ts+21−3View file
| @@ -1,4 +1,4 @@ | ||
| 1 | -import { chmod, mkdir, readFile, writeFile } from "node:fs/promises"; | |
| 1 | +import { chmod, mkdir, readFile, rename, rm, writeFile } from "node:fs/promises"; | |
| 2 | 2 | import { homedir } from "node:os"; |
| 3 | 3 | import path from "node:path"; |
| 4 | 4 | import { CliError, type Io } from "./io.js"; |
| @@ -49,15 +49,31 @@ | ||
| 49 | 49 | export async function writeConfig(env: NodeJS.ProcessEnv, cfg: StoredConfig): Promise<string> { |
| 50 | 50 | const dir = configDir(env); |
| 51 | 51 | await mkdir(dir, { recursive: true, mode: 0o700 }); |
| 52 | + // mkdir's mode only applies to a directory it creates: an existing one may be readable by others. | |
| 53 | + await chmod(dir, 0o700); | |
| 52 | 54 | const file = configPath(env); |
| 53 | 55 | const clean: StoredConfig = {}; |
| 54 | 56 | if (cfg.url) clean.url = cfg.url; |
| 55 | 57 | if (cfg.token) clean.token = cfg.token; |
| 56 | - await writeFile(file, JSON.stringify(clean, null, 2) + "\n", { mode: 0o600 }); | |
| 57 | - await chmod(file, 0o600); // writeFile's mode only applies on create | |
| 58 | + // A new file that is never readable by others, then renamed over the old one, so the token is never | |
| 59 | + // in a loosely permissioned file, even for a moment, and a crash never leaves half a config. | |
| 60 | + const tmp = path.join(dir, `.config.json.${process.pid}.${Date.now()}.tmp`); | |
| 61 | + try { | |
| 62 | + await writeFile(tmp, JSON.stringify(clean, null, 2) + "\n", { mode: 0o600, flag: "wx" }); | |
| 63 | + await rename(tmp, file); | |
| 64 | + } catch (e) { | |
| 65 | + await rm(tmp, { force: true }); | |
| 66 | + throw e; | |
| 67 | + } | |
| 58 | 68 | return file; |
| 59 | 69 | } |
| 60 | 70 | |
| 71 | +/** localhost, *.localhost, 127.0.0.0/8 and ::1. */ | |
| 72 | +export function isLoopback(hostname: string): boolean { | |
| 73 | + const h = hostname.toLowerCase().replace(/^\[|\]$/g, ""); | |
| 74 | + return h === "localhost" || h.endsWith(".localhost") || h === "::1" || /^127(\.\d{1,3}){3}$/.test(h); | |
| 75 | +} | |
| 76 | + | |
| 61 | 77 | /** Normalizes a user-typed server URL: adds https://, drops trailing slashes and any path. */ |
| 62 | 78 | export function normalizeUrl(u: string): string { |
| 63 | 79 | let s = u.trim(); |
| @@ -69,6 +85,8 @@ | ||
| 69 | 85 | throw new CliError(`invalid URL: ${u}`); |
| 70 | 86 | } |
| 71 | 87 | if (parsed.protocol !== "https:" && parsed.protocol !== "http:") throw new CliError(`URL must be http(s): ${u}`); |
| 88 | + // Tokens go with every request: plain http only to this machine (a local development server). | |
| 89 | + if (parsed.protocol === "http:" && !isLoopback(parsed.hostname)) throw new CliError(`refusing ${parsed.protocol}//${parsed.host}: tokens would be sent unencrypted; use https (plain http works only for localhost)`); | |
| 72 | 90 | return `${parsed.protocol}//${parsed.host}${parsed.pathname.replace(/\/+$/, "")}`; |
| 73 | 91 | } |
| 74 | 92 |
modifiedapps/cli/test/config.test.ts+24−1View file
| @@ -1,4 +1,4 @@ | ||
| 1 | -import { mkdirSync, statSync, writeFileSync, readFileSync } from "node:fs"; | |
| 1 | +import { chmodSync, mkdirSync, readdirSync, statSync, writeFileSync, readFileSync } from "node:fs"; | |
| 2 | 2 | import path from "node:path"; |
| 3 | 3 | import { afterEach, beforeEach, describe, expect, it } from "vitest"; |
| 4 | 4 | import { configPath, normalizeUrl, resolveConfig, writeConfig } from "../src/config.js"; |
| @@ -38,11 +38,34 @@ | ||
| 38 | 38 | expect(JSON.parse(readFileSync(file, "utf8"))).toEqual({ url: "https://a.example", token: "dfy_x" }); |
| 39 | 39 | }); |
| 40 | 40 | |
| 41 | + it("tightens an existing loose config directory and file, and leaves no temporary file", async () => { | |
| 42 | + const env = isolatedEnv(); | |
| 43 | + const file = configPath(env); | |
| 44 | + mkdirSync(path.dirname(file), { recursive: true, mode: 0o755 }); | |
| 45 | + chmodSync(path.dirname(file), 0o755); | |
| 46 | + writeFileSync(file, "{}", { mode: 0o644 }); | |
| 47 | + chmodSync(file, 0o644); | |
| 48 | + await writeConfig(env, { url: "https://a.example", token: "dfy_y" }); | |
| 49 | + expect(statSync(path.dirname(file)).mode & 0o777).toBe(0o700); | |
| 50 | + expect(statSync(file).mode & 0o777).toBe(0o600); | |
| 51 | + expect(JSON.parse(readFileSync(file, "utf8"))).toEqual({ url: "https://a.example", token: "dfy_y" }); | |
| 52 | + expect(readdirSync(path.dirname(file))).toEqual(["config.json"]); | |
| 53 | + }); | |
| 54 | + | |
| 41 | 55 | it("normalizes URLs", () => { |
| 42 | 56 | expect(normalizeUrl("forge.example.com/")).toBe("https://forge.example.com"); |
| 43 | 57 | expect(normalizeUrl("http://localhost:8787")).toBe("http://localhost:8787"); |
| 58 | + expect(normalizeUrl("http://127.0.0.1:8787")).toBe("http://127.0.0.1:8787"); | |
| 59 | + expect(normalizeUrl("http://[::1]:8787")).toBe("http://[::1]:8787"); | |
| 60 | + expect(normalizeUrl("http://dev.localhost")).toBe("http://dev.localhost"); | |
| 44 | 61 | expect(() => normalizeUrl("ftp://x")).toThrow(); |
| 45 | 62 | }); |
| 63 | + | |
| 64 | + it("refuses plain http anywhere but this machine, since tokens go with every request", () => { | |
| 65 | + expect(() => normalizeUrl("http://forge.example.com")).toThrow(/unencrypted/); | |
| 66 | + expect(() => normalizeUrl("http://10.0.0.5:8787")).toThrow(/unencrypted/); | |
| 67 | + expect(() => normalizeUrl("http://localhost.example.com")).toThrow(/unencrypted/); | |
| 68 | + }); | |
| 46 | 69 | }); |
| 47 | 70 | |
| 48 | 71 | describe("login / whoami / logout", () => { |
modifiedapps/cli/test/migrate.test.ts+2−0View file
| @@ -150,6 +150,8 @@ | ||
| 150 | 150 | // The request carried the source, resolved owner, keep-visibility and the GitHub token. |
| 151 | 151 | const post = srv.find("POST", "/api/v3/diffinery/imports")[0]!.body as Record<string, unknown>; |
| 152 | 152 | expect(post).toMatchObject({ source: "gh-org/widgets", owner: "alice", visibility: "keep", github_token: "ght" }); |
| 153 | + // The ambient GitHub token goes along, and the command says so. | |
| 154 | + expect(r.stderr).toContain(`note: sending GH_TOKEN to ${srv.url} so it can read gh-org/widgets from GitHub during the import`); | |
| 153 | 155 | // It polled until the import finished, and reported the result and the secrets to set by hand. |
| 154 | 156 | expect(polls).toBeGreaterThanOrEqual(2); |
| 155 | 157 | expect(r.stdout).toContain("Imported gh-org/widgets → alice/widgets"); |
modifiedapps/diffinery/docs/accounts/cli-sign-in.md+1−1View file
| @@ -98,7 +98,7 @@ | ||
| 98 | 98 | - `diffinery logout` revokes it on the server and removes it from the computer. Signing in again on the same computer revokes the token it replaces. |
| 99 | 99 | - Revoke it under **Settings → Access tokens** to sign a lost or old computer out. |
| 100 | 100 | |
| 101 | -The CLI saves the server URL and the token in `~/.config/diffinery/config.json` (or under `$XDG_CONFIG_HOME`), readable only by you. `DIFFINERY_TOKEN` and `DIFFINERY_URL` override the saved values. | |
| 101 | +The CLI saves the server URL and the token in `~/.config/diffinery/config.json` (or under `$XDG_CONFIG_HOME`), readable only by you: the folder is set to `0700` and the file to `0600` each time it is saved. `DIFFINERY_TOKEN` and `DIFFINERY_URL` override the saved values. The server URL must be `https://`, since the token goes with every request; plain `http://` works only for this computer (`localhost`, `127.0.0.1`, `[::1]`), such as a local development server, and anything else fails with `refusing http://<host>: tokens would be sent unencrypted`. | |
| 102 | 102 | |
| 103 | 103 | ## How it stays safe |
| 104 | 104 |
modifiedapps/diffinery/docs/migrate-from-github.md+1−1View file
| @@ -73,7 +73,7 @@ | ||
| 73 | 73 | - It is used only for this import, encrypted at rest, and deleted when the import finishes, fails or is cancelled. A retry needs it again. |
| 74 | 74 | - It is never returned by the API, MCP or the web UI, and never written to logs. In the container, Git gets it from per-host configuration, never on its command line. |
| 75 | 75 | - It is sent only to the one GitHub host you import from. Redirects to another host are not followed, the clone URL must be on that host (`GitHub returned a clone URL on another host`), and release assets get the token only when they are on the API's own origin. |
| 76 | -- The CLI sends `GH_TOKEN` (or `GITHUB_TOKEN`) to Diffinery over HTTPS so the server can use it. | |
| 76 | +- The CLI sends `GH_TOKEN` (or `GITHUB_TOKEN`) to Diffinery over HTTPS so the server can use it, and says so on stderr (`note: sending GH_TOKEN to https://diffinery.com so it can read <owner/repo> from GitHub during the import`). Unset both to import only public data without a token. | |
| 77 | 77 | |
| 78 | 78 | ### GitHub Enterprise Server |
| 79 | 79 |