Organization secrets reach all, private or selected repositories (#91)
Claude-Session: https://claude.ai/code/session_016yhgAu6oKfqes812Yd4qjo Co-authored-by: Claude <noreply@anthropic.com>
Canberk Demirci authored · GitHub committed signed1 parent 0dbc898commit de9b88fabc00de67d84251cbf93b0e8ab1b0286dBrowse files
9 files changed+206−14
Files (9)
- apps/cli/src/commands/admin.ts+14−2
- apps/cli/test/misc.test.ts+24−0
- apps/diffinery/docs/actions/secrets-and-variables.md+23−4
- apps/diffinery/docs/security/audit-log.md+2−1
- apps/diffinery/migrations/0014_org_secret_repos.sql+9−0
- apps/diffinery/src/http/api.ts+80−6
- apps/diffinery/src/lib/directory.ts+1−0
- apps/diffinery/src/repo/actions.ts+8−1
- apps/diffinery/test/e2e/security.test.mjs+45−0
modifiedapps/cli/src/commands/admin.ts+14−2View file
| @@ -16,11 +16,15 @@ | ||
| 16 | 16 | const secretSet: Command = { |
| 17 | 17 | name: "secret set", |
| 18 | 18 | summary: "Create or update an Actions secret", |
| 19 | - description: "The value comes from --body, from stdin when piped, or a hidden prompt.\nRepository level by default; --org sets an organization secret.", | |
| 19 | + description: `The value comes from --body, from stdin when piped, or a hidden prompt. | |
| 20 | +Repository level by default; --org sets an organization secret. An organization secret | |
| 21 | +reaches every repository unless --visibility (or --repos, which implies selected) says otherwise.`, | |
| 20 | 22 | args: "<NAME>", |
| 21 | 23 | options: { |
| 22 | 24 | body: { type: "string", short: "b", desc: "Secret value", arg: "VALUE" }, |
| 23 | 25 | org: { type: "string", short: "o", desc: "Set an organization secret instead", arg: "ORG" }, |
| 26 | + visibility: { type: "string", desc: "Organization secrets: which repositories get it (all, private or selected)", arg: "all|private|selected" }, | |
| 27 | + repos: { type: "string", desc: "Organization secrets: the repositories that get it, comma separated", arg: "a,b" }, | |
| 24 | 28 | }, |
| 25 | 29 | repo: true, |
| 26 | 30 | minArgs: 1, |
| @@ -29,9 +33,17 @@ | ||
| 29 | 33 | checkName(name!, "secret"); |
| 30 | 34 | const org = str(o, "org"); |
| 31 | 35 | const target = org ? `/orgs/${enc(org)}/actions/secrets/${enc(name!)}` : `${repoPath(await ctx.repo())}/actions/secrets/${enc(name!)}`; |
| 36 | + const repos = (str(o, "repos") ?? "").split(",").map((r) => r.trim()).filter(Boolean); | |
| 37 | + const visibility = str(o, "visibility") ?? (repos.length ? "selected" : undefined); | |
| 38 | + if ((visibility || repos.length) && !org) throw new CliError("--visibility and --repos are for organization secrets (--org)", 2); | |
| 39 | + if (visibility && !["all", "private", "selected"].includes(visibility)) throw new CliError("--visibility must be all, private or selected", 2); | |
| 40 | + if (visibility === "selected" && !repos.length) throw new CliError("--visibility selected needs --repos", 2); | |
| 41 | + if (repos.length && visibility !== "selected") throw new CliError("--repos needs --visibility selected", 2); | |
| 32 | 42 | const value = await valueFromFlagOrStdin(ctx.io, str(o, "body"), `Value for ${name}: `); |
| 33 | 43 | if (!value) throw new CliError("secret value is empty"); |
| 34 | - await (await ctx.authedApi()).put(target, { value }); | |
| 44 | + const api = await ctx.authedApi(); | |
| 45 | + const ids = await Promise.all(repos.map(async (r) => (await api.get<{ id: number }>(`/repos/${enc(org!)}/${enc(r.replace(/^.*\//, ""))}`)).id)); | |
| 46 | + await api.put(target, { value, ...(visibility ? { visibility } : {}), ...(ids.length ? { selected_repository_ids: ids } : {}) }); | |
| 35 | 47 | ctx.out(`${ctx.c.green("✓")} Set secret ${name}${org ? ` for ${org}` : ""}`); |
| 36 | 48 | }, |
| 37 | 49 | }; |
modifiedapps/cli/test/misc.test.ts+24−0View file
| @@ -136,3 +136,27 @@ | ||
| 136 | 136 | } |
| 137 | 137 | }); |
| 138 | 138 | }); |
| 139 | + | |
| 140 | +describe("secret set --org", () => { | |
| 141 | + let srv: FakeServer; | |
| 142 | + beforeEach(async () => { | |
| 143 | + srv = await fakeServer(); | |
| 144 | + }); | |
| 145 | + afterEach(() => srv.close()); | |
| 146 | + | |
| 147 | + it("shares an organization secret with the repositories given in --repos", async () => { | |
| 148 | + const env = isolatedEnv({ DIFFINERY_URL: srv.url, DIFFINERY_TOKEN: "dfy_t" }); | |
| 149 | + srv.on("GET", "/api/v3/repos/acme/web", { body: { id: 11, name: "web" } }); | |
| 150 | + srv.on("GET", "/api/v3/repos/acme/api", { body: { id: 22, name: "api" } }); | |
| 151 | + srv.on("PUT", "/api/v3/orgs/acme/actions/secrets/DEPLOY", { status: 204 }); | |
| 152 | + const r = await run(["secret", "set", "DEPLOY", "--org", "acme", "--body", "v", "--repos", "web, acme/api"], { env }); | |
| 153 | + expect(r.code).toBe(0); | |
| 154 | + expect(srv.find("PUT", "/api/v3/orgs/acme/actions/secrets/DEPLOY")[0]!.body).toEqual({ value: "v", visibility: "selected", selected_repository_ids: [11, 22] }); | |
| 155 | + const plain = await run(["secret", "set", "DEPLOY", "--org", "acme", "--body", "v2"], { env }); | |
| 156 | + expect(plain.code).toBe(0); | |
| 157 | + expect(srv.find("PUT", "/api/v3/orgs/acme/actions/secrets/DEPLOY")[1]!.body).toEqual({ value: "v2" }); | |
| 158 | + const bad = await run(["secret", "set", "DEPLOY", "--org", "acme", "--body", "v", "--visibility", "selected"], { env }); | |
| 159 | + expect(bad.code).toBe(2); | |
| 160 | + expect(bad.stderr).toContain("needs --repos"); | |
| 161 | + }); | |
| 162 | +}); |
modifiedapps/diffinery/docs/actions/secrets-and-variables.md+23−4View file
| @@ -10,7 +10,7 @@ | ||
| 10 | 10 | | Kind | Scope | Manage in the web UI | Manage with the CLI and API | Who can manage | |
| 11 | 11 | |---|---|---|---|---| |
| 12 | 12 | | Repository secret | One repository | Yes | Yes | People with the admin role on the repository | |
| 13 | -| Organization secret | Every repository the organization owns | No | Yes | Organization owners and admins, and site admins (people only) | | |
| 13 | +| Organization secret | Every repository the organization owns, its private ones, or the ones you select | No | Yes | Organization owners and admins, and site admins (people only) | | |
| 14 | 14 | | Repository variable | One repository | Yes | Yes | People with the admin role on the repository | |
| 15 | 15 | | Organization variable | Every repository the organization owns | No | API only | Organization owners and admins, and site admins (people only) | |
| 16 | 16 | | Environment secret | Jobs of one repository that use the environment | No | API only | People with the admin role on the repository | |
| @@ -77,10 +77,18 @@ | ||
| 77 | 77 | |
| 78 | 78 | ## Organization secrets |
| 79 | 79 | |
| 80 | -Organization secrets are available to every workflow in every repository the organization owns. They're managed with the CLI or the API; there's no web page for them. | |
| 80 | +Organization secrets are available to workflows in the organization's repositories. As on GitHub, each secret has a `visibility` that says which repositories get it: | |
| 81 | 81 | |
| 82 | +- `all`: every repository the organization owns (the default for a new secret). | |
| 83 | +- `private`: its private and internal repositories, not its public ones. | |
| 84 | +- `selected`: only the repositories you list. | |
| 85 | + | |
| 86 | +They're managed with the CLI or the API; there's no web page for them. | |
| 87 | + | |
| 82 | 88 | ```sh |
| 83 | 89 | diffinery secret set SENTRY_AUTH_TOKEN --org acme |
| 90 | +diffinery secret set DEPLOY_KEY --org acme --repos website,api # only these two repositories | |
| 91 | +diffinery secret set NPM_TOKEN --org acme --visibility private | |
| 84 | 92 | diffinery secret list --org acme |
| 85 | 93 | diffinery secret delete SENTRY_AUTH_TOKEN --org acme |
| 86 | 94 | ``` |
| @@ -92,8 +100,19 @@ | ||
| 92 | 100 | -d '{"value": "sntrys_..."}' |
| 93 | 101 | ``` |
| 94 | 102 | |
| 95 | -`GET /api/v3/orgs/{org}/actions/secrets` lists names and update times, and `DELETE /api/v3/orgs/{org}/actions/secrets/{name}` removes one. Only organization owners and admins (and site admins) can use these endpoints; anyone else gets `403 organization admins only`. | |
| 103 | +With the API, send `visibility` and, for `selected`, `selected_repository_ids` (the repositories' numeric `id`s) with the value: `{"value": "…", "visibility": "selected", "selected_repository_ids": [123, 456]}`. An update that leaves them out keeps the secret's selection. An id that isn't one of the organization's repositories gets `422`. | |
| 96 | 104 | |
| 105 | +| Endpoint | Does | | |
| 106 | +|---|---| | |
| 107 | +| `GET /api/v3/orgs/{org}/actions/secrets` | Lists names, update times and `visibility` | | |
| 108 | +| `GET /api/v3/orgs/{org}/actions/secrets/{name}` | One secret's name, update time and `visibility` (never the value) | | |
| 109 | +| `GET /api/v3/orgs/{org}/actions/secrets/{name}/repositories` | The selected repositories | | |
| 110 | +| `PUT /api/v3/orgs/{org}/actions/secrets/{name}/repositories` | Replaces the selection: `{"selected_repository_ids": [123, 456]}` | | |
| 111 | +| `PUT` or `DELETE /api/v3/orgs/{org}/actions/secrets/{name}/repositories/{repository_id}` | Adds or removes one repository | | |
| 112 | +| `DELETE /api/v3/orgs/{org}/actions/secrets/{name}` | Removes the secret | | |
| 113 | + | |
| 114 | +Changing the selection of a secret whose visibility isn't `selected` gets `409 the secret's visibility isn't selected`. A repository that leaves the selection stops getting the secret from its next job. Only organization owners and admins (and site admins) can use these endpoints; anyone else gets `403 organization admins only`. | |
| 115 | + | |
| 97 | 116 | All three endpoints, listing included, are account-level actions: the CLI or API needs a personal token with the `admin` scope that isn't restricted to repositories. A `read` or `write` token gets `403 this token needs the admin scope`, and a token restricted to repositories gets `403 this token is limited to repositories and can't change account settings`. |
| 98 | 117 | |
| 99 | 118 | ## Variables |
| @@ -272,7 +291,7 @@ | ||
| 272 | 291 | 1. `GITHUB_TOKEN` and `DIFFINERY_TOKEN`: always the job's own token. |
| 273 | 292 | 2. Environment secrets, in a job that uses the environment. |
| 274 | 293 | 3. Repository secrets (in a called reusable workflow: the secrets the caller passed). |
| 275 | -4. Organization secrets. | |
| 294 | +4. Organization secrets shared with the repository (see [Organization secrets](#organization-secrets)). | |
| 276 | 295 | |
| 277 | 296 | For variables, from highest to lowest: |
| 278 | 297 |
modifiedapps/diffinery/docs/security/audit-log.md+2−1View file
| @@ -88,7 +88,8 @@ | ||
| 88 | 88 | | `org.create` | An organization is created from the web UI | the organization's login | none | |
| 89 | 89 | | `org.update` | An organization's settings are changed on its **Settings** tab, or with `PATCH /api/v3/orgs/{org}` | the organization's login | `login` and `base_role` (web UI), or `name` and `base_role` (API) | |
| 90 | 90 | | `org.delete` | An organization is deleted (web or `DELETE /api/v3/orgs/{org}`) | the organization's login | none | |
| 91 | -| `org.secret.set`, `org.secret.delete` | An organization secret is saved or deleted with `PUT`/`DELETE /api/v3/orgs/{org}/actions/secrets/{name}` (or `diffinery secret ... --org`) | the organization's login | `name` (never the value), `via: "api"` | | |
| 91 | +| `org.secret.set`, `org.secret.delete` | An organization secret is saved or deleted with `PUT`/`DELETE /api/v3/orgs/{org}/actions/secrets/{name}` (or `diffinery secret ... --org`) | the organization's login | `name` (never the value), and for `set` its `visibility` and any selected `repositories`; `via: "api"` | | |
| 92 | +| `org.secret.repositories`, `org.secret.repository.add`, `org.secret.repository.remove` | An organization secret's repository selection is replaced, or one repository is added or removed | the organization's login | `name`, and `repositories` or `repository`; `via: "api"` | | |
| 92 | 93 | | `org.variable_set`, `org.variable_delete` | An organization Actions variable is created, changed or deleted with `POST`, `PATCH` or `DELETE /api/v3/orgs/{org}/actions/variables[/{name}]`, or added by a GitHub import of an organization | the organization's login (API), or the GitHub organization (import) | `name` (upper-cased); `via: "api"` for API changes; an import adds `import` (its id) and `org` | |
| 93 | 94 | | `org.invite` | Someone without an account is invited to an organization: an email address entered in **Add people** on the organization's **People** tab, or `POST /api/v3/orgs/{org}/invitations` | the organization's login | `email`, `role` | |
| 94 | 95 | | `org.invite.revoke` | A pending organization invitation is revoked with **Revoke** | the organization's login | `id` (the invitation id) | |
addedapps/diffinery/migrations/0014_org_secret_repos.sql+9−0View file
| @@ -1,0 +1,9 @@ | ||
| 1 | +-- Organization secrets choose which repositories get them, as on GitHub: every repository (`all`), | |
| 2 | +-- private and internal ones (`private`), or the ones listed in org_secret_repos (`selected`). | |
| 3 | +ALTER TABLE org_secrets ADD COLUMN visibility TEXT NOT NULL DEFAULT 'all'; | |
| 4 | +CREATE TABLE org_secret_repos ( | |
| 5 | + org_id TEXT NOT NULL, | |
| 6 | + name TEXT NOT NULL, | |
| 7 | + repo_id TEXT NOT NULL REFERENCES repos(id) ON DELETE CASCADE, | |
| 8 | + PRIMARY KEY (org_id, name, repo_id) | |
| 9 | +); |
modifiedapps/diffinery/src/http/api.ts+80−6View file
| @@ -17,7 +17,7 @@ | ||
| 17 | 17 | import { repoCreated } from "../mail/notify.js"; |
| 18 | 18 | import { actionsRest } from "./actions-runtime.js"; |
| 19 | 19 | import { compat } from "./compat.js"; |
| 20 | -import { baseOf, encPath, ghAutoMerge, ghCheckRun, ghComment, ghCommit, ghGitCommit, ghJob, ghRef, ghRun, ghFile, ghLabel, ghReview, ghReviewComment, ghUser, isoSeconds, nodeId, pageOf, paginate, repoApi, repoHtml, setLinks } from "./gh.js"; | |
| 20 | +import { baseOf, encPath, ghAutoMerge, ghCheckRun, ghComment, ghCommit, ghGitCommit, ghJob, ghRef, ghRun, ghFile, ghLabel, ghReview, ghReviewComment, ghUser, isoSeconds, nodeId, numericId, pageOf, paginate, repoApi, repoHtml, setLinks } from "./gh.js"; | |
| 21 | 21 | import { decodePath, issueCtx, issuesOut, pullDetail, pullsOut, repoOut } from "./ghctx.js"; |
| 22 | 22 | |
| 23 | 23 | /** |
| @@ -931,14 +931,80 @@ | ||
| 931 | 931 | if (role !== "owner" && role !== "admin" && !a.site_admin) throw new HttpError(403, "organization admins only"); |
| 932 | 932 | return { a, org }; |
| 933 | 933 | } |
| 934 | +type OrgSecretRow = { name: string; updated_at: string; visibility: "all" | "private" | "selected" }; | |
| 935 | +const ORG_SECRET_VISIBILITY = ["all", "private", "selected"] as const; | |
| 936 | +const orgSecretJson = (c: AppContext, org: Account, s: OrgSecretRow) => ({ | |
| 937 | + name: s.name, | |
| 938 | + created_at: s.updated_at, | |
| 939 | + updated_at: s.updated_at, | |
| 940 | + visibility: s.visibility, | |
| 941 | + ...(s.visibility === "selected" ? { selected_repository_ids_url: `${c.env.PUBLIC_URL}/api/v3/orgs/${org.login}/actions/secrets/${s.name}/repositories`, selected_repositories_url: `${c.env.PUBLIC_URL}/api/v3/orgs/${org.login}/actions/secrets/${s.name}/repositories` } : {}), | |
| 942 | +}); | |
| 943 | +/** The organization's repositories with the given numeric (GitHub) ids; 422 for any id that isn't one of them. */ | |
| 944 | +async function orgReposById(c: AppContext, org: Account, ids: unknown): Promise<Repo[]> { | |
| 945 | + if (!Array.isArray(ids) || ids.some((x) => typeof x !== "number" && typeof x !== "string")) throw new HttpError(422, "selected_repository_ids must be a list of repository ids"); | |
| 946 | + if (ids.length > 1000) throw new HttpError(422, "at most 1000 repositories can be selected"); | |
| 947 | + const repos = await c.get("dir").reposVisibleTo(requireAccount(c), org.id); | |
| 948 | + const byId = new Map(repos.map((r) => [String(numericId(r.id)), r])); | |
| 949 | + return ids.map((x) => { | |
| 950 | + const r = byId.get(String(x)); | |
| 951 | + if (!r) throw new HttpError(422, `${x} isn't a repository of ${org.login}`); | |
| 952 | + return r; | |
| 953 | + }); | |
| 954 | +} | |
| 955 | +async function orgSecret(c: AppContext, org: Account, name: string): Promise<OrgSecretRow> { | |
| 956 | + const s = await c.env.DB.prepare("SELECT name, updated_at, visibility FROM org_secrets WHERE org_id = ? AND name = ?").bind(org.id, name.toUpperCase()).first<OrgSecretRow>(); | |
| 957 | + if (!s) throw new HttpError(404, "secret not found"); | |
| 958 | + return s; | |
| 959 | +} | |
| 960 | +async function setSecretRepos(c: AppContext, org: Account, name: string, repos: Repo[]): Promise<void> { | |
| 961 | + await c.env.DB.batch([ | |
| 962 | + c.env.DB.prepare("DELETE FROM org_secret_repos WHERE org_id = ? AND name = ?").bind(org.id, name), | |
| 963 | + ...repos.map((r) => c.env.DB.prepare("INSERT OR IGNORE INTO org_secret_repos (org_id, name, repo_id) VALUES (?, ?, ?)").bind(org.id, name, r.id)), | |
| 964 | + ]); | |
| 965 | +} | |
| 934 | 966 | api.get("/orgs/:org/actions/secrets", async (c) => { |
| 935 | 967 | const { org } = await orgAdmin(c); |
| 936 | - const { results } = await c.env.DB.prepare("SELECT name, updated_at FROM org_secrets WHERE org_id = ? ORDER BY name").bind(org.id).all<{ name: string; updated_at: string }>(); | |
| 937 | - return c.json({ total_count: results.length, secrets: results }); | |
| 968 | + const { results } = await c.env.DB.prepare("SELECT name, updated_at, visibility FROM org_secrets WHERE org_id = ? ORDER BY name").bind(org.id).all<OrgSecretRow>(); | |
| 969 | + return c.json({ total_count: results.length, secrets: results.map((s) => orgSecretJson(c, org, s)) }); | |
| 938 | 970 | }); |
| 971 | +api.get("/orgs/:org/actions/secrets/:name", async (c) => { | |
| 972 | + const { org } = await orgAdmin(c); | |
| 973 | + return c.json(orgSecretJson(c, org, await orgSecret(c, org, c.req.param("name")))); | |
| 974 | +}); | |
| 975 | +// GitHub's repository selection for a secret with `visibility: selected`. | |
| 976 | +api.get("/orgs/:org/actions/secrets/:name/repositories", async (c) => { | |
| 977 | + const { org } = await orgAdmin(c); | |
| 978 | + const s = await orgSecret(c, org, c.req.param("name")); | |
| 979 | + const { results } = await c.env.DB.prepare("SELECT r.* FROM org_secret_repos x JOIN repos r ON r.id = x.repo_id WHERE x.org_id = ? AND x.name = ? ORDER BY r.name").bind(org.id, s.name).all<{ id: string }>(); | |
| 980 | + const repos = (await Promise.all(results.map((r) => c.get("dir").repoById(r.id)))).filter((r): r is Repo => !!r); | |
| 981 | + return c.json({ total_count: repos.length, repositories: repos.map((r) => repoJson(c, r)) }); | |
| 982 | +}); | |
| 983 | +api.put("/orgs/:org/actions/secrets/:name/repositories", async (c) => { | |
| 984 | + const { a, org } = await orgAdmin(c); | |
| 985 | + const s = await orgSecret(c, org, c.req.param("name")); | |
| 986 | + if (s.visibility !== "selected") throw new HttpError(409, "the secret's visibility isn't selected"); | |
| 987 | + const repos = await orgReposById(c, org, (await body<{ selected_repository_ids?: unknown }>(c)).selected_repository_ids); | |
| 988 | + await setSecretRepos(c, org, s.name, repos); | |
| 989 | + c.executionCtx.waitUntil(c.get("dir").audit({ actorId: a.id, actorLogin: a.login, action: "org.secret.repositories", target: org.login, detail: { name: s.name, repositories: repos.map((r) => r.name), via: "api" } })); | |
| 990 | + return c.body(null, 204); | |
| 991 | +}); | |
| 992 | +for (const method of ["put", "delete"] as const) { | |
| 993 | + api[method]("/orgs/:org/actions/secrets/:name/repositories/:repoId", async (c) => { | |
| 994 | + const { a, org } = await orgAdmin(c); | |
| 995 | + const s = await orgSecret(c, org, c.req.param("name")); | |
| 996 | + if (s.visibility !== "selected") throw new HttpError(409, "the secret's visibility isn't selected"); | |
| 997 | + const [repo] = await orgReposById(c, org, [c.req.param("repoId")]); | |
| 998 | + if (method === "put") await c.env.DB.prepare("INSERT OR IGNORE INTO org_secret_repos (org_id, name, repo_id) VALUES (?, ?, ?)").bind(org.id, s.name, repo!.id).run(); | |
| 999 | + else await c.env.DB.prepare("DELETE FROM org_secret_repos WHERE org_id = ? AND name = ? AND repo_id = ?").bind(org.id, s.name, repo!.id).run(); | |
| 1000 | + c.executionCtx.waitUntil(c.get("dir").audit({ actorId: a.id, actorLogin: a.login, action: method === "put" ? "org.secret.repository.add" : "org.secret.repository.remove", target: org.login, detail: { name: s.name, repository: repo!.name, via: "api" } })); | |
| 1001 | + return c.body(null, 204); | |
| 1002 | + }); | |
| 1003 | +} | |
| 939 | 1004 | api.delete("/orgs/:org/actions/secrets/:name", async (c) => { |
| 940 | 1005 | const { a, org } = await orgAdmin(c); |
| 941 | 1006 | const r = await c.env.DB.prepare("DELETE FROM org_secrets WHERE org_id = ? AND name = ?").bind(org.id, c.req.param("name").toUpperCase()).run(); |
| 1007 | + await c.env.DB.prepare("DELETE FROM org_secret_repos WHERE org_id = ? AND name = ?").bind(org.id, c.req.param("name").toUpperCase()).run(); | |
| 942 | 1008 | // Like GitHub: deleting a secret that doesn't exist is a 404, and nothing is audited. |
| 943 | 1009 | if (!r.meta?.changes) throw new HttpError(404, "secret not found"); |
| 944 | 1010 | c.executionCtx.waitUntil(c.get("dir").audit({ actorId: a.id, actorLogin: a.login, action: "org.secret.delete", target: org.login, detail: { name: c.req.param("name").toUpperCase(), via: "api" } })); |
| @@ -948,9 +1014,17 @@ | ||
| 948 | 1014 | const { a, org } = await orgAdmin(c); |
| 949 | 1015 | const name = c.req.param("name").toUpperCase(); |
| 950 | 1016 | if (!/^[A-Z_][A-Z0-9_]*$/.test(name)) throw new HttpError(422, "invalid secret name"); |
| 951 | - const { value } = await body<{ value: string }>(c); | |
| 952 | - await c.env.DB.prepare("INSERT OR REPLACE INTO org_secrets (org_id, name, ciphertext, updated_at) VALUES (?, ?, ?, ?)").bind(org.id, name, await encryptSecret(c.env.MASTER_KEY, `org:${org.id}`, value), new Date().toISOString()).run(); | |
| 953 | - c.executionCtx.waitUntil(c.get("dir").audit({ actorId: a.id, actorLogin: a.login, action: "org.secret.set", target: org.login, detail: { name, via: "api" } })); | |
| 1017 | + const b = await body<{ value: string; visibility?: unknown; selected_repository_ids?: unknown }>(c); | |
| 1018 | + if (b.visibility !== undefined && !ORG_SECRET_VISIBILITY.includes(b.visibility as OrgSecretRow["visibility"])) throw new HttpError(422, "visibility must be all, private or selected"); | |
| 1019 | + if (b.selected_repository_ids !== undefined && b.visibility !== "selected") throw new HttpError(422, "selected_repository_ids needs visibility selected"); | |
| 1020 | + const existing = await c.env.DB.prepare("SELECT visibility FROM org_secrets WHERE org_id = ? AND name = ?").bind(org.id, name).first<{ visibility: OrgSecretRow["visibility"] }>(); | |
| 1021 | + // A new secret reaches every repository unless the request says otherwise; an update keeps its selection. | |
| 1022 | + const visibility = (b.visibility as OrgSecretRow["visibility"] | undefined) ?? existing?.visibility ?? "all"; | |
| 1023 | + const repos = b.selected_repository_ids !== undefined ? await orgReposById(c, org, b.selected_repository_ids) : null; | |
| 1024 | + await c.env.DB.prepare("INSERT OR REPLACE INTO org_secrets (org_id, name, ciphertext, updated_at, visibility) VALUES (?, ?, ?, ?, ?)").bind(org.id, name, await encryptSecret(c.env.MASTER_KEY, `org:${org.id}`, b.value), new Date().toISOString(), visibility).run(); | |
| 1025 | + if (repos) await setSecretRepos(c, org, name, repos); | |
| 1026 | + else if (visibility !== "selected") await c.env.DB.prepare("DELETE FROM org_secret_repos WHERE org_id = ? AND name = ?").bind(org.id, name).run(); | |
| 1027 | + c.executionCtx.waitUntil(c.get("dir").audit({ actorId: a.id, actorLogin: a.login, action: "org.secret.set", target: org.login, detail: { name, visibility, ...(repos ? { repositories: repos.map((r) => r.name) } : {}), via: "api" } })); | |
| 954 | 1028 | return c.body(null, 204); |
| 955 | 1029 | }); |
| 956 | 1030 |
modifiedapps/diffinery/src/lib/directory.ts+1−0View file
| @@ -201,6 +201,7 @@ | ||
| 201 | 201 | this.db.prepare("DELETE FROM org_members WHERE org_id = ?").bind(org.id), |
| 202 | 202 | this.db.prepare("DELETE FROM invites WHERE org_id = ? AND accepted_at IS NULL").bind(org.id), |
| 203 | 203 | this.db.prepare("DELETE FROM org_secrets WHERE org_id = ?").bind(org.id), |
| 204 | + this.db.prepare("DELETE FROM org_secret_repos WHERE org_id = ?").bind(org.id), | |
| 204 | 205 | this.db.prepare("DELETE FROM org_variables WHERE org_id = ?").bind(org.id), |
| 205 | 206 | this.db.prepare("DELETE FROM login_redirects WHERE account_id = ?").bind(org.id), |
| 206 | 207 | this.db.prepare("UPDATE accounts SET login = ?, status = 'disabled' WHERE id = ?").bind(`deleted-${org.id}`, org.id), |
modifiedapps/diffinery/src/repo/actions.ts+8−1View file
| @@ -1524,7 +1524,14 @@ | ||
| 1524 | 1524 | private async orgSecrets(): Promise<Record<string, string>> { |
| 1525 | 1525 | const m = this.repo.meta; |
| 1526 | 1526 | if (m.ownerKind !== "org") return {}; |
| 1527 | - const rows = await this.env.DB.prepare("SELECT name, ciphertext FROM org_secrets WHERE org_id = ?").bind(m.ownerId).all<{ name: string; ciphertext: string }>(); | |
| 1527 | + // Only the secrets whose repository selection includes this repository (visibility all, private or selected). | |
| 1528 | + const rows = await this.env.DB.prepare( | |
| 1529 | + `SELECT s.name, s.ciphertext FROM org_secrets s WHERE s.org_id = ?1 AND (s.visibility = 'all' | |
| 1530 | + OR (s.visibility = 'private' AND EXISTS (SELECT 1 FROM repos r WHERE r.id = ?2 AND r.visibility != 'public')) | |
| 1531 | + OR (s.visibility = 'selected' AND EXISTS (SELECT 1 FROM org_secret_repos x WHERE x.org_id = s.org_id AND x.name = s.name AND x.repo_id = ?2)))`, | |
| 1532 | + ) | |
| 1533 | + .bind(m.ownerId, m.repoId) | |
| 1534 | + .all<{ name: string; ciphertext: string }>(); | |
| 1528 | 1535 | const out: Record<string, string> = {}; |
| 1529 | 1536 | for (const r of rows.results) out[r.name] = await decryptSecret(this.env.MASTER_KEY, `org:${m.ownerId}`, r.ciphertext); |
| 1530 | 1537 | return out; |
modifiedapps/diffinery/test/e2e/security.test.mjs+45−0View file
| @@ -1936,3 +1936,48 @@ | ||
| 1936 | 1936 | await status(204, "DELETE", `${R}/actions/runners/${m1.id}`); |
| 1937 | 1937 | await status(401, "POST", `${R}/actions/runners/claim`, {}, m1.token); |
| 1938 | 1938 | }); |
| 1939 | + | |
| 1940 | +test("security: an organization secret reaches only the repositories it's shared with", async () => { | |
| 1941 | + // Three of guild's repositories: two private, one public. | |
| 1942 | + const repos = {}; | |
| 1943 | + for (const [name, priv] of [["sec-a", true], ["sec-b", true], ["sec-pub", false]]) { | |
| 1944 | + repos[name] = await ok("POST", "/orgs/guild/repos", { name, private: priv }); | |
| 1945 | + await ok("PUT", `/repos/guild/${name}/contents/README.md`, { message: "init", content: b64(`# ${name}\n`) }); | |
| 1946 | + } | |
| 1947 | + await status(204, "PUT", "/orgs/guild/actions/secrets/EVERYWHERE", { value: "e" }); | |
| 1948 | + await status(204, "PUT", "/orgs/guild/actions/secrets/PRIVATE_ONLY", { value: "p", visibility: "private" }); | |
| 1949 | + await status(204, "PUT", "/orgs/guild/actions/secrets/ONLY_A", { value: "a", visibility: "selected", selected_repository_ids: [repos["sec-a"].id] }); | |
| 1950 | + await status(422, "PUT", "/orgs/guild/actions/secrets/BAD", { value: "x", visibility: "selected", selected_repository_ids: [12345] }); | |
| 1951 | + await status(422, "PUT", "/orgs/guild/actions/secrets/BAD", { value: "x", visibility: "some" }); | |
| 1952 | + const listed = await ok("GET", "/orgs/guild/actions/secrets"); | |
| 1953 | + const vis = Object.fromEntries(listed.secrets.map((s) => [s.name, s.visibility])); | |
| 1954 | + assert.deepEqual([vis.EVERYWHERE, vis.PRIVATE_ONLY, vis.ONLY_A], ["all", "private", "selected"]); | |
| 1955 | + assert.deepEqual((await ok("GET", "/orgs/guild/actions/secrets/ONLY_A/repositories")).repositories.map((r) => r.name), ["sec-a"]); | |
| 1956 | + // Updating the value keeps the selection. | |
| 1957 | + await status(204, "PUT", "/orgs/guild/actions/secrets/ONLY_A", { value: "a2" }); | |
| 1958 | + await status(409, "PUT", `/orgs/guild/actions/secrets/EVERYWHERE/repositories/${repos["sec-a"].id}`); | |
| 1959 | + | |
| 1960 | + // Each repository's job asks for every secret by name and gets only what it's shared. | |
| 1961 | + const wf = "name: ci\non: push\njobs:\n build:\n runs-on: [self-hosted, linux]\n steps:\n - run: echo '${{ toJSON(secrets) }}'\n"; | |
| 1962 | + const got = async (name, write = true) => { | |
| 1963 | + if (write) await ok("PUT", `/repos/guild/${name}/contents/.github/workflows/ci.yml`, { message: "ci", content: b64(wf) }); | |
| 1964 | + let names; | |
| 1965 | + await until(`${name}'s job claimed`, async () => { | |
| 1966 | + const r = await api("POST", `/repos/guild/${name}/actions/runners/claim`, { name: "org-secrets", labels: ["linux"] }); | |
| 1967 | + if (r.status !== 200) return false; | |
| 1968 | + const spec = await (await fetch(`${base}/runner/jobs/${r.json.job_id}`, { headers: { authorization: `Bearer ${r.json.token}` } })).json(); | |
| 1969 | + names = Object.keys(spec.secrets).filter((k) => !/_TOKEN$/.test(k)).sort(); | |
| 1970 | + await fetch(`${base}/runner/jobs/${r.json.job_id}/complete`, { method: "POST", headers: { authorization: `Bearer ${r.json.token}`, "content-type": "application/json" }, body: JSON.stringify({ conclusion: "success", outputs: {} }) }); | |
| 1971 | + return true; | |
| 1972 | + }); | |
| 1973 | + return names; | |
| 1974 | + }; | |
| 1975 | + assert.deepEqual(await got("sec-a"), ["EVERYWHERE", "ONLY_A", "PRIVATE_ONLY"]); | |
| 1976 | + assert.deepEqual(await got("sec-b"), ["EVERYWHERE", "PRIVATE_ONLY"]); | |
| 1977 | + assert.deepEqual(await got("sec-pub"), ["EVERYWHERE"]); | |
| 1978 | + | |
| 1979 | + // Adding sec-b to the selection shares it from the next job. | |
| 1980 | + await status(204, "PUT", `/orgs/guild/actions/secrets/ONLY_A/repositories/${repos["sec-b"].id}`); | |
| 1981 | + await ok("PUT", "/repos/guild/sec-b/contents/again.txt", { message: "again", content: b64("again\n") }); | |
| 1982 | + assert.deepEqual(await got("sec-b", false), ["EVERYWHERE", "ONLY_A", "PRIVATE_ONLY"]); | |
| 1983 | +}); |