diff --git a/docs/runbooks/setup.md b/docs/runbooks/setup.md index dfcd9fa..c1c64c5 100644 --- a/docs/runbooks/setup.md +++ b/docs/runbooks/setup.md @@ -120,16 +120,19 @@ that branch. Start the GitHub App manifest flow: ```bash -npm run setup:app -- --write-env +npm run setup:app ``` For an organization-owned App: ```bash -npm run setup:app -- --org --write-env +npm run setup:app -- --org ``` Keep the command running while the user completes the browser flow. +The command saves credentials to `.env` without printing their values. On +POSIX systems, it makes new and existing `.env` files readable and writable +only by the owner. The `--write-env` flag is still accepted for compatibility. ### Human checkpoint: create the App diff --git a/scripts/lib/env-file.ts b/scripts/lib/env-file.ts index f0c2462..e380662 100644 --- a/scripts/lib/env-file.ts +++ b/scripts/lib/env-file.ts @@ -1,4 +1,4 @@ -import { readFile, writeFile } from "node:fs/promises"; +import { chmod, readFile, writeFile } from "node:fs/promises"; /** * Sets each key in dotenv text: replaces an existing `KEY=` line (the first, commented-out lines untouched) or @@ -19,6 +19,8 @@ function upsertEnv(text: string, values: Readonly>): stri export async function writeEnvFile(path: string, values: Readonly>): Promise { let current = ""; try { + // writeFile's mode only protects new files. Restrict an existing file before adding secrets. + if (process.platform !== "win32") await chmod(path, 0o600); current = await readFile(path, "utf8"); } catch (error) { if ((error as NodeJS.ErrnoException).code !== "ENOENT") throw error; diff --git a/scripts/setup-app.ts b/scripts/setup-app.ts index 5e7dedd..1d54aa6 100644 --- a/scripts/setup-app.ts +++ b/scripts/setup-app.ts @@ -13,14 +13,15 @@ import { githubApiUrl } from "../app/github/auth.js"; import { writeEnvFile } from "./lib/env-file.js"; import { githubWebUrl } from "./lib/github.js"; import { buildManifest, exchangeManifestCode, formPage, newState, registrationUrl, startCallbackServer } from "./lib/manifest.js"; -import { bold, dim, errorMessage, green, yellow } from "./lib/output.js"; +import { bold, dim, errorMessage, green } from "./lib/output.js"; const { values } = parseArgs({ options: { org: { type: "string" }, name: { type: "string" }, port: { type: "string" }, - "write-env": { type: "boolean", default: false }, + // Accepted for compatibility; credentials are always saved instead of printed. + "write-env": { type: "boolean", default: true }, }, }); @@ -67,17 +68,8 @@ console.log(`\n${bold("1. Install the App")} on every repository in factory.conf console.log(` ${app.html_url}/installations/new`); for (const repo of repos) console.log(` - ${repo}`); -if (values["write-env"]) { - await writeEnvFile(".env", env); - console.log(`\n${green("✓")} Wrote GITHUB_APP_ID, GITHUB_APP_PRIVATE_KEY, and GITHUB_WEBHOOK_SECRET to .env`); -} else { - console.log(`\n${bold("Add these to .env")}; setup:render, setup:github, and doctor read them from there:`); - console.log(` GITHUB_APP_ID=${env.GITHUB_APP_ID}`); - console.log(` GITHUB_APP_PRIVATE_KEY=${env.GITHUB_APP_PRIVATE_KEY}`); - console.log(` GITHUB_WEBHOOK_SECRET=${env.GITHUB_WEBHOOK_SECRET}`); - console.log(dim(" The private key is base64-encoded so it fits on one line. Keep these values secret.")); - console.log(yellow(" The private key isn't saved anywhere else. Rerun with --write-env to write .env instead.")); -} +await writeEnvFile(".env", env); +console.log(`\n${green("✓")} Wrote GITHUB_APP_ID, GITHUB_APP_PRIVATE_KEY, and GITHUB_WEBHOOK_SECRET to .env`); console.log(`\n${bold("2. Create the Render env groups")} after adding RENDER_API_KEY, RENDER_WORKSPACE_ID, and ANTHROPIC_API_KEY to .env:`); console.log(" npm run setup:render"); diff --git a/tests/setup-app-secrets.test.ts b/tests/setup-app-secrets.test.ts new file mode 100644 index 0000000..e2780fa --- /dev/null +++ b/tests/setup-app-secrets.test.ts @@ -0,0 +1,82 @@ +import { chmod, mkdtemp, readFile, rm, stat, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { writeEnvFile } from "../scripts/lib/env-file.js"; + +vi.mock("../app/config/index.js", () => ({ + config: { + repos: { "acme/web": {} }, + labels: { triage: "factory:triage", specApproved: "factory:spec-approved", prReview: "factory:pr-review", autopilot: "factory:autopilot" }, + }, +})); +vi.mock("../scripts/lib/env-file.js", () => ({ writeEnvFile: vi.fn() })); +vi.mock("../scripts/lib/manifest.js", async (importOriginal) => ({ + ...(await importOriginal()), + startCallbackServer: async () => ({ + url: "http://127.0.0.1:5555", + result: Promise.resolve({ + id: 123, + slug: "sf-test", + name: "sf-test", + pem: "fake-private-key-do-not-print", + html_url: "https://github.com/apps/sf-test", + }), + close: async () => {}, + }), +})); + +const originalArgv = process.argv; +const directories: string[] = []; + +afterEach(async () => { + process.argv = originalArgv; + vi.restoreAllMocks(); + vi.clearAllMocks(); + await Promise.all(directories.splice(0).map((path) => rm(path, { recursive: true, force: true }))); +}); + +describe("setup:app credential output", () => { + it.each([{ args: [] }, { args: ["--write-env"] }])("saves credentials without printing them with arguments $args", async ({ args }) => { + process.argv = ["node", "scripts/setup-app.ts", ...args]; + const log = vi.spyOn(console, "log").mockImplementation(() => {}); + vi.resetModules(); + await import("../scripts/setup-app.js"); + + const [path, credentials] = vi.mocked(writeEnvFile).mock.calls[0]; + expect(path).toBe(".env"); + expect(credentials).toEqual({ + GITHUB_APP_ID: "123", + GITHUB_APP_PRIVATE_KEY: Buffer.from("fake-private-key-do-not-print").toString("base64"), + GITHUB_WEBHOOK_SECRET: expect.stringMatching(/^[a-f0-9]{64}$/), + }); + const output = log.mock.calls.flat().join("\n"); + expect(output).toContain("Wrote GITHUB_APP_ID, GITHUB_APP_PRIVATE_KEY, and GITHUB_WEBHOOK_SECRET to .env"); + expect(output).toContain("https://github.com/apps/sf-test/installations/new"); + expect(output).not.toContain("fake-private-key-do-not-print"); + expect(output).not.toContain(credentials.GITHUB_APP_PRIVATE_KEY); + expect(output).not.toContain(credentials.GITHUB_WEBHOOK_SECRET); + }); +}); + +describe("dotenv credential storage", () => { + it.each(["new", "existing"])("protects $0 dotenv files and preserves unrelated settings", async (kind) => { + const existing = kind === "existing"; + const directory = await mkdtemp(join(tmpdir(), "software-factory-env-")); + directories.push(directory); + const path = join(directory, ".env"); + if (existing) { + await writeFile(path, "# Keep this comment\nOTHER_SETTING=keep\nGITHUB_APP_PRIVATE_KEY=old\n"); + if (process.platform !== "win32") await chmod(path, 0o644); + } + const { writeEnvFile: save } = await vi.importActual("../scripts/lib/env-file.js"); + await save(path, { GITHUB_APP_PRIVATE_KEY: "new-private-key", GITHUB_WEBHOOK_SECRET: "new-webhook-secret" }); + + const contents = await readFile(path, "utf8"); + expect(contents).toContain("GITHUB_APP_PRIVATE_KEY=new-private-key\n"); + expect(contents).toContain("GITHUB_WEBHOOK_SECRET=new-webhook-secret\n"); + expect(contents).not.toContain("GITHUB_APP_PRIVATE_KEY=old"); + if (existing) expect(contents).toContain("# Keep this comment\nOTHER_SETTING=keep\n"); + if (process.platform !== "win32") expect((await stat(path)).mode & 0o777).toBe(0o600); + }); +});