Jobs get only the secrets they name, agent runs get none unless allowed, and adding by email always invites (#77)
* A job's runner gets only the secrets the job names, as on GitHub Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016yhgAu6oKfqes812Yd4qjo * Runs of agent code get no repository or organization secrets unless an admin allows it Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016yhgAu6oKfqes812Yd4qjo * Adding someone by email always sends an invitation, so the address doesn't reveal their username and they agree first Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016yhgAu6oKfqes812Yd4qjo --------- Co-authored-by: Claude <noreply@anthropic.com>
Canberk Demirci authored · GitHub committed signed1 parent e0fc0c6commit 2805acb6e67ce91e77b15f1e4be093c1b11235b1Browse files
16 files changed+284−70
Files (16)
- apps/diffinery/docs/accounts/invitations.md+8−9
- apps/diffinery/docs/accounts/organizations.md+2−3
- apps/diffinery/docs/actions/secrets-and-variables.md+5−2
- apps/diffinery/docs/actions/workflow-permissions.md+6−3
- apps/diffinery/src/http/actions-runtime.ts+3−3
- apps/diffinery/src/lib/directory.ts+21−13
- apps/diffinery/src/repo/actions.ts+7−3
- apps/diffinery/src/repo/job-secrets.ts+57−0
- apps/diffinery/src/repo/token-policy.ts+7−3
- apps/diffinery/src/web/account.tsx+2−2
- apps/diffinery/src/web/auth.tsx+21−17
- apps/diffinery/src/web/repo-settings.tsx+15−3
- apps/diffinery/test/e2e/e2e.test.mjs+4−4
- apps/diffinery/test/e2e/security.test.mjs+96−3
- apps/diffinery/test/unit/actions-runtime.test.ts+2−2
- apps/diffinery/test/unit/job-secrets.test.ts+28−0
modifiedapps/diffinery/docs/accounts/invitations.md+8−9View file
| @@ -51,8 +51,8 @@ | ||
| 51 | 51 | |
| 52 | 52 | | You entered | What happens | |
| 53 | 53 | | --- | --- | |
| 54 | -| A username, or the email address of an existing account | They join right away, with no invitation to accept: "Added diffy to acme as member. We let them know." They get an email and an in-app notification. | | |
| 55 | -| An email address without an account | Diffinery emails an invitation. The page shows **Invitation sent to friend@example.com** with the link and a copy button, and the note "They don't have an account yet, so we emailed them an invitation. You can also send them this link yourself. It works once and expires in 14 days: opening it creates their account and passkey and adds them here." | | |
| 54 | +| A username | They join right away, with no invitation to accept: "Added diffy to acme as member. We let them know." They get an email and an in-app notification. | | |
| 55 | +| An email address | Diffinery emails an invitation, whether or not the address has an account, so the form never tells you who an address belongs to and nobody joins without agreeing. The page shows **Invitation sent to friend@example.com** with the link and a copy button, and the note "We emailed them an invitation. You can also send them this link yourself. It works once and expires in 14 days: they join when they accept it, creating an account and passkey first if they don't have one." | | |
| 56 | 56 | |
| 57 | 57 | The invitation then appears under **Pending invitations** on the same tab, with who sent it, when, the expiry date and the role. See [Organizations](/docs/accounts/organizations#add-people) for every message the form can show. |
| 58 | 58 | |
| @@ -78,8 +78,8 @@ | ||
| 78 | 78 | |
| 79 | 79 | | You entered | What happens | |
| 80 | 80 | | --- | --- | |
| 81 | -| The username of a person or an agent, or the email address of an existing account | They get the role right away as a direct collaborator: "diffy now has write access. We let them know." People get an email and an in-app notification. If they already were a collaborator, their role changes to the one you chose. | | |
| 82 | -| An email address without an account | Diffinery emails an invitation and shows **Invitation sent to friend@example.com** with the link to send yourself. When the person joins, they get that role on the repository. | | |
| 81 | +| The username of a person or an agent | They get the role right away as a direct collaborator: "diffy now has write access. We let them know." People get an email and an in-app notification. If they already were a collaborator, their role changes to the one you chose. | | |
| 82 | +| An email address | Diffinery emails an invitation, whether or not the address has an account, and shows **Invitation sent to friend@example.com** with the link to send yourself. When the person accepts it, they get that role on the repository. | | |
| 83 | 83 | |
| 84 | 84 | | Message | Cause | |
| 85 | 85 | | --- | --- | |
| @@ -119,7 +119,7 @@ | ||
| 119 | 119 | | `-r, --role ROLE` | `member` (default), `admin` or `owner`. Needs `--org`. | |
| 120 | 120 | | `--json` | Print the API response (`id` and `url`) as JSON. | |
| 121 | 121 | |
| 122 | -The command prints the invite link and reminds you that it is valid for 14 days. Unlike the web form, `diffinery invite` always creates an invitation, even when the address already has an account (they accept it with **Accept invite**; see [Accept an invite with an existing account](#accept-an-invite-with-an-existing-account)). It refuses an address that already has a pending invitation to the organization, and an address whose account is already a member of it (see [API errors](#api-errors)). The CLI can't create repository invites; use the repository's **Settings → Collaborators** page. See [CLI reference](/docs/reference/cli) for every command. | |
| 122 | +The command prints the invite link and reminds you that it is valid for 14 days. Like the web form, `diffinery invite` always creates an invitation, even when the address already has an account (they accept it with **Accept invite**; see [Accept an invite with an existing account](#accept-an-invite-with-an-existing-account)). It refuses an address that already has a pending invitation to the organization (see [API errors](#api-errors)). The CLI can't create repository invites; use the repository's **Settings → Collaborators** page. See [CLI reference](/docs/reference/cli) for every command. | |
| 123 | 123 | |
| 124 | 124 | ## Invite with the API |
| 125 | 125 | |
| @@ -147,7 +147,7 @@ | ||
| 147 | 147 | { "id": "3f1c0e8a9b7d4c2e8f6a5b4c3d2e1f0a", "url": "https://diffinery.com/invite/..." } |
| 148 | 148 | ``` |
| 149 | 149 | |
| 150 | -The caller must be an owner or admin of the organization, or a site administrator. Only owners (and site administrators) can invite with the `owner` role. Like the CLI, this endpoint always creates an invitation; it doesn't add existing accounts directly. It refuses an address that already has a pending invitation to the organization (compared without regard to letter case) and the address of an account that is already a member. | |
| 150 | +The caller must be an owner or admin of the organization, or a site administrator. Only owners (and site administrators) can invite with the `owner` role. Like the CLI and the web form, this endpoint always creates an invitation; it doesn't add existing accounts directly. It refuses an address that already has a pending invitation to the organization (compared without regard to letter case). An address whose account is already a member still gets an invitation, so the answer never tells you whose address it is; accepting it changes nothing. | |
| 151 | 151 | |
| 152 | 152 | ### Instance invite |
| 153 | 153 | |
| @@ -177,7 +177,6 @@ | ||
| 177 | 177 | | 422 | `a valid email is required` | The `email` field is missing or isn't an email address. | |
| 178 | 178 | | 422 | `role must be member, admin or owner` | `role` has another value. | |
| 179 | 179 | | 422 | `friend@example.com already has a pending invitation; revoke it first to send a new one` | The address already has an open invitation to this organization (or, for `POST /api/v3/invitations`, an open instance invitation). Letter case doesn't matter. Revoke the organization invitation on the **People** tab first. | |
| 180 | -| 422 | `diffy is already a member of this organization` | The address belongs to an account that is already a member. Change their role on the **People** tab instead. | | |
| 181 | 180 | |
| 182 | 181 | Error bodies are JSON: `{"message": "...", "documentation_url": "..."}`. |
| 183 | 182 | |
| @@ -195,7 +194,7 @@ | ||
| 195 | 194 | |
| 196 | 195 | The email lists who invited you, the organization or repository and the role (for organization and repository invites) and the expiry date, and has an **Accept invitation** button. It says the invitation only works for that email address. Invite emails can't be turned off, because they are the only way to reach someone without an account. |
| 197 | 196 | |
| 198 | -If the address already belongs to a Diffinery account (an invitation from the CLI or API), the email goes to that account. | |
| 197 | +If the address already belongs to a Diffinery account, the email goes to that account. | |
| 199 | 198 | |
| 200 | 199 | ## Invite link rules |
| 201 | 200 | |
| @@ -229,7 +228,7 @@ | ||
| 229 | 228 | |
| 230 | 229 | If the invited address already has an account: |
| 231 | 230 | |
| 232 | -1. Open the invite link. The page says the address already has an account and shows a **Sign in** button. | |
| 231 | +1. Open the invite link from the email. The page says the address already has an account and shows a **Sign in** button. (A copy of the link without the email's proof, such as the one the inviter sees, asks you to confirm your email instead, and also offers **Sign in** to accept.) | |
| 233 | 232 | 2. Sign in with your passkey. Diffinery brings you back to the invite. |
| 234 | 233 | 3. Click **Accept invite**. |
| 235 | 234 |
modifiedapps/diffinery/docs/accounts/organizations.md+2−3View file
| @@ -196,10 +196,9 @@ | ||
| 196 | 196 | | You entered | Result | |
| 197 | 197 | | --- | --- | |
| 198 | 198 | | The username of a person | They become a member right away, with no invitation to accept. You see "Added diffy to acme as member. We let them know." They get an email ("You're now a member of acme", naming who added them) and an in-app notification ("<your username> added you to acme as a member"). | |
| 199 | -| The email address of someone who already has an account | The same as entering their username: they are added right away. | | |
| 200 | -| An email address with no account | Diffinery creates an invitation and emails it. The page shows **Invitation sent to friend@example.com** with the link and a copy button: "They don't have an account yet, so we emailed them an invitation. You can also send them this link yourself. It works once and expires in 14 days: opening it creates their account and passkey and adds them here." | | |
| 199 | +| An email address | Diffinery creates an invitation and emails it, whether or not the address has an account: the form doesn't reveal who an address belongs to, and they join only by accepting. The page shows **Invitation sent to friend@example.com** with the link and a copy button: "We emailed them an invitation. You can also send them this link yourself. It works once and expires in 14 days: they join when they accept it, creating an account and passkey first if they don't have one." | | |
| 201 | 200 | |
| 202 | -A person whose account is waiting for an invite (pending) becomes active when you add them by username or email. See [Invitations](/docs/accounts/invitations) for how invitations work, including inviting from the CLI and API. | |
| 201 | +A person whose account is waiting for an invite (pending) becomes active when you add them by username or invite their email address. See [Invitations](/docs/accounts/invitations) for how invitations work, including inviting from the CLI and API. | |
| 203 | 202 | |
| 204 | 203 | | Message | Cause | |
| 205 | 204 | | --- | --- | |
modifiedapps/diffinery/docs/actions/secrets-and-variables.md+5−2View file
| @@ -263,6 +263,7 @@ | ||
| 263 | 263 | - A secret or variable that doesn't exist evaluates to an empty string. |
| 264 | 264 | - Values are read when each job starts, so changing a secret affects jobs that start afterwards, including later jobs of a run that's in progress. |
| 265 | 265 | - A reusable workflow only receives the secrets its caller passes (a mapping or `secrets: inherit`). See [Reusable workflows](/docs/actions/reusable-workflows). |
| 266 | +- A job's runner receives only the secrets the job names in its expressions (`secrets.NAME` or `secrets['NAME']`, in any letter case, anywhere in the job or in the workflow's `env`), as on GitHub, plus `ACTIONS_STEP_DEBUG` and `ACTIONS_RUNNER_DEBUG`. Other secrets never reach the runner, so a compromised action or tool in one job can't read them. A job that uses `secrets` as a whole, such as `toJSON(secrets)` or an index computed at run time, gets all of them. | |
| 266 | 267 | |
| 267 | 268 | ### Precedence |
| 268 | 269 | |
| @@ -287,8 +288,10 @@ | ||
| 287 | 288 | |
| 288 | 289 | This is a difference from GitHub worth understanding. GitHub withholds secrets from `pull_request` runs of pull requests that come from forks. Diffinery has no forks: every pull request comes from a branch in the same repository, pushed by someone with write access, so `pull_request` runs get the repository's secrets, and they run the workflow files from the pull request's own test merge commit. The exception is a pull request [imported from GitHub](/docs/migrate-from-github) whose head was in a fork (its head shows as `owner:branch`): its runs get no secrets and a read-only token, as on GitHub. `pull_request_target` runs the base branch's workflow on the base commit, also with secrets, so a pull request can't change what it runs. |
| 289 | 290 | |
| 290 | -In practice: **anyone who can push a branch can run a workflow that uses the repository's secrets**, including agents with write access. Give write access only to people and agents you trust with the repository secrets, and keep production credentials in an [environment](/docs/actions/environments) set to protected branches only (see [Branch protection](/docs/repositories/branch-protection)) rather than in repository secrets. Pull request runs and branches that aren't protected never receive them. | |
| 291 | +Runs of code an agent wrote are the other exception. A run whose trigger was an agent, whose branch an agent pushed last, or whose pull request an agent opened or pushed to gets no repository or organization secrets, only its job token. An agent with write access could otherwise push a workflow that sends them anywhere. A repository admin who is a person can allow it under **Settings → Actions** (**Give runs of agent code the repository's and organization's secrets**), or with `PUT /repos/{owner}/{repo}/actions/permissions/workflow` and `"agent_runs_use_secrets": true`. The change is audited. Environment secrets follow the environment's own rules. | |
| 291 | 292 | |
| 293 | +In practice: **anyone who can push a branch can run a workflow that uses the repository's secrets**, and so can agents with write access where an admin allows it. Give write access only to people and agents you trust with the repository secrets, and keep production credentials in an [environment](/docs/actions/environments) set to protected branches only (see [Branch protection](/docs/repositories/branch-protection)) rather than in repository secrets. Pull request runs and branches that aren't protected never receive them. | |
| 294 | + | |
| 292 | 295 | ## Masking in logs |
| 293 | 296 | |
| 294 | 297 | The runner replaces every secret value with `***` before log lines leave the runner, so secrets don't appear in job logs, step names, step summaries or failure messages. |
| @@ -415,7 +418,7 @@ | ||
| 415 | 418 | | Shortest value that is masked | 3 characters | |
| 416 | 419 | | Organization variable value | 48 KB (49,152 characters) | |
| 417 | 420 | | Number of organization variables | No limit of its own | |
| 418 | -| Number and size of secrets | Diffinery doesn't enforce a limit of its own; every secret is decrypted for every job, so keep them few and small | | |
| 421 | +| Number and size of secrets | Diffinery doesn't enforce a limit of its own; every secret is decrypted when each job starts (only the ones the job names are sent to its runner), so keep them few and small | | |
| 419 | 422 | |
| 420 | 423 | ## Troubleshooting |
| 421 | 424 |
modifiedapps/diffinery/docs/actions/workflow-permissions.md+6−3View file
| @@ -130,9 +130,11 @@ | ||
| 130 | 130 | |
| 131 | 131 | A second setting, `can_approve_pull_request_reviews`, decides whether job tokens may open pull requests and approve or request changes on them (see [Creating and approving pull requests](#creating-and-approving-pull-requests)). It's `false` by default. |
| 132 | 132 | |
| 133 | +A third, `agent_runs_use_secrets` (a Diffinery addition), decides whether runs of code an agent wrote get the repository's and organization's secrets. It's `false` by default; see [Which runs receive secrets](/docs/actions/secrets-and-variables#which-runs-receive-secrets). | |
| 134 | + | |
| 133 | 135 | ### Read and change the setting |
| 134 | 136 | |
| 135 | -In the web UI, open the repository's **Settings → Actions** tab (`/{owner}/{repo}/settings/actions`). Under **Workflow permissions**, pick **Read repository contents and packages permissions** or **Read and write permissions**, check or clear **Allow GitHub Actions to create and approve pull requests**, and click **Save**. You see "Actions settings saved." The tab needs the **admin** role on the repository. | |
| 137 | +In the web UI, open the repository's **Settings → Actions** tab (`/{owner}/{repo}/settings/actions`). Under **Workflow permissions**, pick **Read repository contents and packages permissions** or **Read and write permissions**, check or clear **Allow GitHub Actions to create and approve pull requests** and **Give runs of agent code the repository's and organization's secrets**, and click **Save**. You see "Actions settings saved." The tab needs the **admin** role on the repository. | |
| 136 | 138 | |
| 137 | 139 | You can also read and change it with the REST API or the CLI. Both need the **admin** role on the repository, and only people can change it: an agent's token gets `403 only people can change what workflows are allowed to do`. |
| 138 | 140 | |
| @@ -146,11 +148,12 @@ | ||
| 146 | 148 | ```json |
| 147 | 149 | { |
| 148 | 150 | "default_workflow_permissions": "read", |
| 149 | - "can_approve_pull_request_reviews": false | |
| 151 | + "can_approve_pull_request_reviews": false, | |
| 152 | + "agent_runs_use_secrets": false | |
| 150 | 153 | } |
| 151 | 154 | ``` |
| 152 | 155 | |
| 153 | -Change it with `PUT`. Send one or both fields; a field you leave out keeps its value. The response is `204 No Content`: | |
| 156 | +Change it with `PUT`. Send any of the fields; a field you leave out keeps its value. The response is `204 No Content`: | |
| 154 | 157 | |
| 155 | 158 | ```sh |
| 156 | 159 | curl -fsS -X PUT https://diffinery.com/api/v3/repos/acme/website/actions/permissions/workflow \ |
modifiedapps/diffinery/src/http/actions-runtime.ts+3−3View file
| @@ -614,7 +614,7 @@ | ||
| 614 | 614 | }); |
| 615 | 615 | |
| 616 | 616 | // The workflow token's defaults: GitHub's "Workflow permissions" settings. |
| 617 | -const workflowSettingsJson = (s: { defaultPermissions: "read" | "write"; canApprovePullRequests: boolean }) => ({ default_workflow_permissions: s.defaultPermissions, can_approve_pull_request_reviews: s.canApprovePullRequests }); | |
| 617 | +const workflowSettingsJson = (s: { defaultPermissions: "read" | "write"; canApprovePullRequests: boolean; agentRunSecrets: boolean }) => ({ default_workflow_permissions: s.defaultPermissions, can_approve_pull_request_reviews: s.canApprovePullRequests, agent_runs_use_secrets: s.agentRunSecrets }); | |
| 618 | 618 | |
| 619 | 619 | actionsRest.get("/repos/:owner/:repo/actions/permissions/workflow", async (c) => { |
| 620 | 620 | const { stub } = await loadRepo(c, P(c).owner, P(c).repo, "admin"); |
| @@ -623,10 +623,10 @@ | ||
| 623 | 623 | |
| 624 | 624 | actionsRest.put("/repos/:owner/:repo/actions/permissions/workflow", async (c) => { |
| 625 | 625 | const { caller, stub } = await loadRepo(c, P(c).owner, P(c).repo, "admin"); |
| 626 | - const b = (await c.req.json().catch(() => null)) as { default_workflow_permissions?: unknown; can_approve_pull_request_reviews?: unknown } | null; | |
| 626 | + const b = (await c.req.json().catch(() => null)) as { default_workflow_permissions?: unknown; can_approve_pull_request_reviews?: unknown; agent_runs_use_secrets?: unknown } | null; | |
| 627 | 627 | if (!b || typeof b !== "object") throw new HttpError(422, "a JSON body is required"); |
| 628 | 628 | // Audited by the repository (an audit event per change), marked as made through the API. |
| 629 | - await stub.setWorkflowSettings({ defaultPermissions: b.default_workflow_permissions as "read" | "write" | undefined, canApprovePullRequests: b.can_approve_pull_request_reviews as boolean | undefined }, caller, { via: "api" }); | |
| 629 | + await stub.setWorkflowSettings({ defaultPermissions: b.default_workflow_permissions as "read" | "write" | undefined, canApprovePullRequests: b.can_approve_pull_request_reviews as boolean | undefined, agentRunSecrets: b.agent_runs_use_secrets as boolean | undefined }, caller, { via: "api" }); | |
| 630 | 630 | return c.body(null, 204); |
| 631 | 631 | }); |
| 632 | 632 |
modifiedapps/diffinery/src/lib/directory.ts+21−13View file
| @@ -243,16 +243,22 @@ | ||
| 243 | 243 | } |
| 244 | 244 | |
| 245 | 245 | /** |
| 246 | - * Add someone to an organization by username or email, like GitHub's "Add people": people with an | |
| 247 | - * account join right away and get a heads-up; an email without an account gets an invitation that | |
| 248 | - * creates the account and joins the organization. | |
| 246 | + * Add someone to an organization by username or email, like GitHub's "Add people": a username joins | |
| 247 | + * right away and gets a heads-up; an email address gets an invitation that joins the organization | |
| 248 | + * when accepted (creating the account if there is none). | |
| 249 | 249 | */ |
| 250 | 250 | async addOrgPerson(org: Account, by: Account, who: string, role: "owner" | "admin" | "member"): Promise<{ added: Account } | { invited: { email: string; url: string } }> { |
| 251 | 251 | const q = who.trim().replace(/^@/, ""); |
| 252 | 252 | if (!q) throw new HttpError(422, "enter a username or an email address"); |
| 253 | 253 | const email = q.includes("@"); |
| 254 | 254 | if (email && !/^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(q)) throw new HttpError(422, "enter a valid email address"); |
| 255 | - const user = email ? await this.accountByEmail(q) : await this.accountByLogin(q); | |
| 255 | + // An email address always gets an invitation, even when it belongs to an account: adding the | |
| 256 | + // account directly would tell the admin its username, and the person hasn't agreed to join. | |
| 257 | + if (email) { | |
| 258 | + const inv = await this.createInvite(by, { email: q, orgId: org.id, role }); | |
| 259 | + return { invited: { email: q, url: inv.url } }; | |
| 260 | + } | |
| 261 | + const user = await this.accountByLogin(q); | |
| 256 | 262 | if (user) { |
| 257 | 263 | if (user.kind !== "user") throw new HttpError(422, `${user.login} is not a person; agents belong to an organization through Settings → Agents`); |
| 258 | 264 | if (user.status === "disabled") throw new HttpError(422, `${user.login}'s account is disabled`); |
| @@ -262,18 +268,21 @@ | ||
| 262 | 268 | await this.setOrgMember(org.id, user.id, role, by); |
| 263 | 269 | return { added: user }; |
| 264 | 270 | } |
| 265 | - if (!email) throw new HttpError(404, `no one has the username ${q}; to invite someone new, enter their email address`); | |
| 266 | - const inv = await this.createInvite(by, { email: q, orgId: org.id, role }); | |
| 267 | - return { invited: { email: q, url: inv.url } }; | |
| 271 | + throw new HttpError(404, `no one has the username ${q}; to invite someone new, enter their email address`); | |
| 268 | 272 | } |
| 269 | 273 | |
| 270 | - /** Add a collaborator by username (a person or an agent) or email; a new email gets an invitation. */ | |
| 274 | + /** Add a collaborator by username (a person or an agent), or invite an email address. */ | |
| 271 | 275 | async addRepoPerson(repo: Repo, by: Account, who: string, role: Role): Promise<{ added: Account } | { invited: { email: string; url: string } }> { |
| 272 | 276 | const q = who.trim().replace(/^@/, ""); |
| 273 | 277 | if (!q) throw new HttpError(422, "enter a username or an email address"); |
| 274 | 278 | const email = q.includes("@"); |
| 275 | 279 | if (email && !/^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(q)) throw new HttpError(422, "enter a valid email address"); |
| 276 | - const account = email ? await this.accountByEmail(q) : await this.accountByLogin(q); | |
| 280 | + // As for organizations, an email address always gets an invitation. | |
| 281 | + if (email) { | |
| 282 | + const inv = await this.createInvite(by, { email: q, repoId: repo.id, role }); | |
| 283 | + return { invited: { email: q, url: inv.url } }; | |
| 284 | + } | |
| 285 | + const account = await this.accountByLogin(q); | |
| 277 | 286 | if (account) { |
| 278 | 287 | if (account.kind === "org") throw new HttpError(422, `${account.login} is an organization; add its people instead`); |
| 279 | 288 | if (account.status === "disabled") throw new HttpError(422, `${account.login}'s account is disabled`); |
| @@ -282,9 +291,7 @@ | ||
| 282 | 291 | if (account.id !== by.id) await this.notify([account.id], { repoId: repo.id, kind: "repo_access", title: `${by.login} gave you ${role} access to ${repo.owner_login}/${repo.name}`, url: `/${repo.owner_login}/${repo.name}` }); |
| 283 | 292 | return { added: account }; |
| 284 | 293 | } |
| 285 | - if (!email) throw new HttpError(404, `no one has the username ${q}; to invite someone new, enter their email address`); | |
| 286 | - const inv = await this.createInvite(by, { email: q, repoId: repo.id, role }); | |
| 287 | - return { invited: { email: q, url: inv.url } }; | |
| 294 | + throw new HttpError(404, `no one has the username ${q}; to invite someone new, enter their email address`); | |
| 288 | 295 | } |
| 289 | 296 | |
| 290 | 297 | /** Revoke a pending invitation to an organization, a repository, or (neither) the site. False if there was none. */ |
| @@ -420,8 +427,9 @@ | ||
| 420 | 427 | .bind(input.email, input.orgId ?? null, input.repoId ?? null, now()) |
| 421 | 428 | .first(); |
| 422 | 429 | if (pending) throw new HttpError(422, `${input.email} already has a pending invitation; revoke it first to send a new one`); |
| 430 | + // An address that already belongs to a member still gets an invitation (accepting it changes | |
| 431 | + // nothing): refusing would tell the inviter whose address it is. | |
| 423 | 432 | const existing = await this.accountByEmail(input.email); |
| 424 | - if (existing && input.orgId && (await this.orgRole(input.orgId, existing.id))) throw new HttpError(422, `${existing.login} is already a member of this organization`); | |
| 425 | 433 | const token = randomToken(24); |
| 426 | 434 | const id = newId(); |
| 427 | 435 | await this.db |
modifiedapps/diffinery/src/repo/actions.ts+7−3View file
| @@ -31,6 +31,7 @@ | ||
| 31 | 31 | import { ReusableError, expandReusable, localKey, parseCallRef, type Expanded, type Source } from "./reusable.js"; |
| 32 | 32 | import { ancestors, combineResults, jobStatus, runnerLabels, type JobNode } from "./job-graph.js"; |
| 33 | 33 | import { schedulerStub } from "../lib/schedules.js"; |
| 34 | +import { referencedSecrets, RUNNER_SECRETS, secretsFor } from "./job-secrets.js"; | |
| 34 | 35 | |
| 35 | 36 | /** How long a sandbox token works after the sandbox starts. */ |
| 36 | 37 | export const SANDBOX_TOKEN_TTL_MS = 24 * 3600_000; |
| @@ -1189,8 +1190,8 @@ | ||
| 1189 | 1190 | const short = localKey(job.job_key, parent); |
| 1190 | 1191 | let scope: { inputs: Record<string, unknown>; secrets: Record<string, string> }; |
| 1191 | 1192 | try { |
| 1192 | - // A fork's code never sees the repository's secrets, as on GitHub. | |
| 1193 | - scope = await this.callContext(run, exp, parent, !this.untrusted(run)); | |
| 1193 | + // A fork's code never sees the repository's secrets, as on GitHub, and an agent's only where an admin allows it. | |
| 1194 | + scope = await this.callContext(run, exp, parent, !this.untrusted(run) && (!run.agent_origin || this.repo.policy.settings().agentRunSecrets)); | |
| 1194 | 1195 | } catch (e) { |
| 1195 | 1196 | await this.completeJob(jobId, { conclusion: "failure", outputs: {}, failure: { message: (e as Error).message } }); |
| 1196 | 1197 | throw new HttpError(409, (e as Error).message); |
| @@ -1216,7 +1217,10 @@ | ||
| 1216 | 1217 | this.sql.exec("UPDATE jobs SET status = 'in_progress', started_at = ? WHERE id = ?", t, jobId); |
| 1217 | 1218 | this.repo.upsertCheck({ sha: run.head_sha ?? run.sha, name: this.checkName(run, job), status: "in_progress", runId: run.id, jobId, detailsUrl: `${this.env.PUBLIC_URL}/${m.owner}/${m.name}/actions/runs/${run.id}/job/${jobId}` }); |
| 1218 | 1219 | this.repo.broadcast(`run:${run.id}`, { action: "job", jobId }); |
| 1219 | - const secrets: Record<string, string> = { ...scope.secrets, ...(this.untrusted(run) ? {} : environment.secrets), GITHUB_TOKEN: token, DIFFINERY_TOKEN: token }; | |
| 1220 | + // Only the secrets the job's expressions name reach the runner, as on GitHub (plus the runner's own debug switches). | |
| 1221 | + const used = referencedSecrets(def, wf.env); | |
| 1222 | + if (used !== "all") for (const name of RUNNER_SECRETS) used.add(name); | |
| 1223 | + const secrets: Record<string, string> = { ...secretsFor({ ...scope.secrets, ...(this.untrusted(run) ? {} : environment.secrets) }, used), GITHUB_TOKEN: token, DIFFINERY_TOKEN: token }; | |
| 1220 | 1224 | // The job token can do what the effective `permissions:` allow (job level wins, like GitHub). |
| 1221 | 1225 | const permissions = permissionsForJob(exp, job.job_key, { untrusted: this.untrusted(run), repoDefault: this.repo.policy.settings().defaultPermissions }); |
| 1222 | 1226 | this.repo.runtime.grant(jobId, permissions); |
addedapps/diffinery/src/repo/job-secrets.ts+57−0View file
| @@ -1,0 +1,57 @@ | ||
| 1 | +/** | |
| 2 | + * Which secrets a job uses. As on GitHub, a runner gets only the secrets the | |
| 3 | + * job's expressions name, so a compromised action or tool in one job can't read | |
| 4 | + * every secret out of the runner's memory or environment. | |
| 5 | + */ | |
| 6 | + | |
| 7 | +/** Secrets the runner reads itself: they turn on debug logging. */ | |
| 8 | +export const RUNNER_SECRETS = ["ACTIONS_STEP_DEBUG", "ACTIONS_RUNNER_DEBUG"]; | |
| 9 | + | |
| 10 | +const EXPRESSION = /\$\{\{([^]*?)\}\}/g; | |
| 11 | +const SECRETS = /\bsecrets\b/gi; | |
| 12 | +const NAMED = /^\s*(?:\.\s*([A-Za-z_][A-Za-z0-9_]*)|\[\s*(['"])([A-Za-z_][A-Za-z0-9_]*)\2\s*\])/; | |
| 13 | + | |
| 14 | +/** Secret names (upper case) referenced in `expr`, or "all" when it reads `secrets` as a whole (toJSON, a computed index). */ | |
| 15 | +function scan(expr: string, into: Set<string>): boolean { | |
| 16 | + for (const m of expr.matchAll(SECRETS)) { | |
| 17 | + // `steps.secrets.outputs.x` is a property named secrets, not the context. | |
| 18 | + if (/\.\s*$/.test(expr.slice(0, m.index))) continue; | |
| 19 | + const named = NAMED.exec(expr.slice(m.index + m[0].length)); | |
| 20 | + if (!named) return false; | |
| 21 | + into.add((named[1] ?? named[3])!.toUpperCase()); | |
| 22 | + } | |
| 23 | + return true; | |
| 24 | +} | |
| 25 | + | |
| 26 | +/** | |
| 27 | + * The secret names `parts` (a job definition, the workflow's `env` and `defaults`) reference, | |
| 28 | + * upper case; or "all" when an expression uses `secrets` in a way that can't be narrowed. | |
| 29 | + * `if:` values are expressions without `${{ }}`. | |
| 30 | + */ | |
| 31 | +export function referencedSecrets(...parts: unknown[]): Set<string> | "all" { | |
| 32 | + const names = new Set<string>(); | |
| 33 | + let all = false; | |
| 34 | + const walk = (v: unknown, key: string | null): void => { | |
| 35 | + if (all) return; | |
| 36 | + if (typeof v === "string") { | |
| 37 | + if (key === "if" && !v.includes("${{")) { | |
| 38 | + if (!scan(v, names)) all = true; | |
| 39 | + return; | |
| 40 | + } | |
| 41 | + for (const m of v.matchAll(EXPRESSION)) if (!scan(m[1]!, names)) all = true; | |
| 42 | + return; | |
| 43 | + } | |
| 44 | + if (Array.isArray(v)) for (const x of v) walk(x, null); | |
| 45 | + else if (v && typeof v === "object") for (const [k, x] of Object.entries(v)) walk(x, k); | |
| 46 | + }; | |
| 47 | + for (const p of parts) walk(p, null); | |
| 48 | + return all ? "all" : names; | |
| 49 | +} | |
| 50 | + | |
| 51 | +/** `secrets` narrowed to what the job references. */ | |
| 52 | +export function secretsFor(secrets: Record<string, string>, used: Set<string> | "all"): Record<string, string> { | |
| 53 | + if (used === "all") return secrets; | |
| 54 | + const out: Record<string, string> = {}; | |
| 55 | + for (const [name, value] of Object.entries(secrets)) if (used.has(name.toUpperCase())) out[name] = value; | |
| 56 | + return out; | |
| 57 | +} |
modifiedapps/diffinery/src/repo/token-policy.ts+7−3View file
| @@ -16,14 +16,17 @@ | ||
| 16 | 16 | * advisory reviews) hold for code an agent wrote, whoever triggered the run. |
| 17 | 17 | * - Workflow files: a job token never creates or changes them (GitHub's |
| 18 | 18 | * `workflows` permission, which GITHUB_TOKEN cannot have). |
| 19 | + * - Secrets in runs of agent code: none from the repository or organization | |
| 20 | + * unless an admin allows it (`agentRunSecrets`), since an agent with write | |
| 21 | + * access could otherwise push a workflow that sends them anywhere. | |
| 19 | 22 | */ |
| 20 | 23 | |
| 21 | -export type WorkflowSettings = { defaultPermissions: "read" | "write"; canApprovePullRequests: boolean }; | |
| 24 | +export type WorkflowSettings = { defaultPermissions: "read" | "write"; canApprovePullRequests: boolean; agentRunSecrets: boolean }; | |
| 22 | 25 | |
| 23 | 26 | const SETTINGS_KEY = "actions_workflow_settings"; |
| 24 | 27 | /** Repositories that predate the setting keep GitHub's old default. */ |
| 25 | -const LEGACY: WorkflowSettings = { defaultPermissions: "write", canApprovePullRequests: false }; | |
| 26 | -const NEW_REPO: WorkflowSettings = { defaultPermissions: "read", canApprovePullRequests: false }; | |
| 28 | +const LEGACY: WorkflowSettings = { defaultPermissions: "write", canApprovePullRequests: false, agentRunSecrets: false }; | |
| 29 | +const NEW_REPO: WorkflowSettings = { defaultPermissions: "read", canApprovePullRequests: false, agentRunSecrets: false }; | |
| 27 | 30 | |
| 28 | 31 | export const WORKFLOW_DIRS = [".github/workflows", ".diffinery/workflows"]; |
| 29 | 32 | |
| @@ -54,6 +57,7 @@ | ||
| 54 | 57 | if (caller.kind !== "user") throw new HttpError(403, "only people can change what workflows are allowed to do"); |
| 55 | 58 | if (patch.defaultPermissions !== undefined && patch.defaultPermissions !== "read" && patch.defaultPermissions !== "write") throw new HttpError(422, "default_workflow_permissions must be read or write"); |
| 56 | 59 | if (patch.canApprovePullRequests !== undefined && typeof patch.canApprovePullRequests !== "boolean") throw new HttpError(422, "can_approve_pull_request_reviews must be a boolean"); |
| 60 | + if (patch.agentRunSecrets !== undefined && typeof patch.agentRunSecrets !== "boolean") throw new HttpError(422, "agent_runs_use_secrets must be a boolean"); | |
| 57 | 61 | const next = { ...this.settings(), ...Object.fromEntries(Object.entries(patch).filter(([, v]) => v !== undefined)) } as WorkflowSettings; |
| 58 | 62 | this.sql.exec("INSERT OR REPLACE INTO meta (key, value) VALUES (?, ?)", SETTINGS_KEY, JSON.stringify(next)); |
| 59 | 63 | const m = this.repo.meta; |
modifiedapps/diffinery/src/web/account.tsx+2−2View file
| @@ -1570,7 +1570,7 @@ | ||
| 1570 | 1570 | <div class="box" style="border-color:var(--amber)"> |
| 1571 | 1571 | <div class="body"> |
| 1572 | 1572 | <h3>Invitation sent to {email}</h3> |
| 1573 | - <p class="muted">They don't have an account yet, so we emailed them an invitation. You can also send them this link yourself. It works once and expires in 14 days: opening it creates their account and passkey and adds them here.</p> | |
| 1573 | + <p class="muted">We emailed them an invitation. You can also send them this link yourself. It works once and expires in 14 days: they join when they accept it, creating an account and passkey first if they don't have one.</p> | |
| 1574 | 1574 | <div class="row nowrap"> |
| 1575 | 1575 | <div class="secret grow">{url}</div> |
| 1576 | 1576 | <CopyButton text={url} size="base" /> |
| @@ -1756,7 +1756,7 @@ | ||
| 1756 | 1756 | </div> |
| 1757 | 1757 | <button class="btn primary">Add</button> |
| 1758 | 1758 | </div> |
| 1759 | - <div class="hint">People who already have an account join right away and get a heads-up by email and on Diffinery. Anyone else gets an email invitation: opening it creates their account and adds them to {acct.login}.</div> | |
| 1759 | + <div class="hint">A username joins right away and gets a heads-up by email and on Diffinery. An email address gets an invitation: they join {acct.login} when they accept it, creating an account first if they don't have one.</div> | |
| 1760 | 1760 | </div> |
| 1761 | 1761 | </Form> |
| 1762 | 1762 | ) : null} |
modifiedapps/diffinery/src/web/auth.tsx+21−17View file
| @@ -135,8 +135,28 @@ | ||
| 135 | 135 | const auth = new Auth(c.env); |
| 136 | 136 | const inv = await auth.invite(token); |
| 137 | 137 | if (!inv) throw new HttpError(404, "this invite is invalid, already used, or expired"); |
| 138 | - const exists = inv.email ? await c.get("dir").accountByEmail(inv.email) : null; | |
| 139 | 138 | const next = `/invite/${encodeURIComponent(token)}`; |
| 139 | + // The link alone isn't enough to create the account: the copy emailed to the address carries a proof. | |
| 140 | + // Ask for it before saying whether the address has an account, since the inviter has the link too. | |
| 141 | + const proof = c.req.query("proof"); | |
| 142 | + if (inv.email && !(await validInviteProof(c.env, inv.hash, proof))) { | |
| 143 | + return render( | |
| 144 | + c, | |
| 145 | + { title: "Join Diffinery" }, | |
| 146 | + <AuthCard title="Confirm your email"> | |
| 147 | + <p class="muted"> | |
| 148 | + This invitation is for <b>{inv.email}</b>. To make sure it's you, open the link in the invitation email sent to that address. If you can't find it, we can send it again. | |
| 149 | + </p> | |
| 150 | + <Form action={`/invite/${encodeURIComponent(token)}/email`} t={csrfToken(c)}> | |
| 151 | + <button class="btn primary block">Email me the link</button> | |
| 152 | + </Form> | |
| 153 | + <p class="muted small"> | |
| 154 | + Already have an account? <a href={`/login?next=${encodeURIComponent(next)}`}>Sign in</a> to accept. | |
| 155 | + </p> | |
| 156 | + </AuthCard>, | |
| 157 | + ); | |
| 158 | + } | |
| 159 | + const exists = inv.email ? await c.get("dir").accountByEmail(inv.email) : null; | |
| 140 | 160 | if (!inv.email || exists) { |
| 141 | 161 | return render( |
| 142 | 162 | c, |
| @@ -157,22 +177,6 @@ | ||
| 157 | 177 | </AuthCard>, |
| 158 | 178 | ); |
| 159 | 179 | } |
| 160 | - // The link alone isn't enough to create the account: the copy emailed to the address carries a proof. | |
| 161 | - const proof = c.req.query("proof"); | |
| 162 | - if (!(await validInviteProof(c.env, inv.hash, proof))) { | |
| 163 | - return render( | |
| 164 | - c, | |
| 165 | - { title: "Join Diffinery" }, | |
| 166 | - <AuthCard title="Confirm your email"> | |
| 167 | - <p class="muted"> | |
| 168 | - This invitation is for <b>{inv.email}</b>. To make sure it's you, open the link in the invitation email sent to that address. If you can't find it, we can send it again. | |
| 169 | - </p> | |
| 170 | - <Form action={`/invite/${encodeURIComponent(token)}/email`} t={csrfToken(c)}> | |
| 171 | - <button class="btn primary block">Email me the link</button> | |
| 172 | - </Form> | |
| 173 | - </AuthCard>, | |
| 174 | - ); | |
| 175 | - } | |
| 176 | 180 | return render( |
| 177 | 181 | c, |
| 178 | 182 | { title: "Join Diffinery" }, |
modifiedapps/diffinery/src/web/repo-settings.tsx+15−3View file
| @@ -336,7 +336,7 @@ | ||
| 336 | 336 | </> |
| 337 | 337 | )} |
| 338 | 338 | </p> |
| 339 | - {invite ? <Once title={`Invitation sent to ${invite.email}`} value={invite.url} note="They don't have an account yet, so we emailed them an invitation. You can also send them this link yourself. It works once and expires in 14 days: opening it creates their account and passkey and gives them access." /> : null} | |
| 339 | + {invite ? <Once title={`Invitation sent to ${invite.email}`} value={invite.url} note="We emailed them an invitation. You can also send them this link yourself. It works once and expires in 14 days: they get access when they accept it, creating an account and passkey first if they don't have one." /> : null} | |
| 340 | 340 | <Form action={`${base}/settings/access/people`} t={t} class="box"> |
| 341 | 341 | <div class="head"> |
| 342 | 342 | <h3>{raw(ICONS.userPlus)} Add people</h3> |
| @@ -353,7 +353,7 @@ | ||
| 353 | 353 | </div> |
| 354 | 354 | <button class="btn primary">Add</button> |
| 355 | 355 | </div> |
| 356 | - <div class="hint">People and agents with an account get access right away and a heads-up. Anyone else gets an email invitation: opening it creates their account with access to this repository.</div> | |
| 356 | + <div class="hint">A username (a person or an agent) gets access right away and a heads-up. An email address gets an invitation: they get access when they accept it, creating an account first if they don't have one.</div> | |
| 357 | 357 | </div> |
| 358 | 358 | </Form> |
| 359 | 359 | <div class="box"> |
| @@ -492,6 +492,18 @@ | ||
| 492 | 492 | } |
| 493 | 493 | /> |
| 494 | 494 | </div> |
| 495 | + <div class="field"> | |
| 496 | + <KCheckbox | |
| 497 | + name="agent_secrets" | |
| 498 | + checked={settings.agentRunSecrets} | |
| 499 | + label={ | |
| 500 | + <> | |
| 501 | + Give runs of agent code the repository's and organization's secrets | |
| 502 | + <span class="muted small"> — off: a run whose code an agent pushed, or whose pull request an agent opened, gets only its job token. Environment secrets follow the environment's own rules.</span> | |
| 503 | + </> | |
| 504 | + } | |
| 505 | + /> | |
| 506 | + </div> | |
| 495 | 507 | <button class="btn primary">Save</button> |
| 496 | 508 | </div> |
| 497 | 509 | </Form> |
| @@ -688,7 +700,7 @@ | ||
| 688 | 700 | `${repoUrl(l.repo)}/settings/actions`, |
| 689 | 701 | async () => { |
| 690 | 702 | const defaultPermissions = f.s("default_permissions") === "write" ? "write" : "read"; |
| 691 | - await l.stub.setWorkflowSettings({ defaultPermissions, canApprovePullRequests: f.s("can_approve") === "1" }, l.caller); | |
| 703 | + await l.stub.setWorkflowSettings({ defaultPermissions, canApprovePullRequests: f.s("can_approve") === "1", agentRunSecrets: f.s("agent_secrets") === "1" }, l.caller); | |
| 692 | 704 | }, |
| 693 | 705 | "Actions settings saved.", |
| 694 | 706 | ); |
modifiedapps/diffinery/test/e2e/e2e.test.mjs+4−4View file
| @@ -1023,7 +1023,7 @@ | ||
| 1023 | 1023 | await ok("POST", "/user/repos", { name: "policy" }); |
| 1024 | 1024 | const settingsPath = `/repos/${owner}/policy/actions/permissions/workflow`; |
| 1025 | 1025 | // New repositories start like GitHub's: read-only token, no pull request approvals. |
| 1026 | - assert.deepEqual(await ok("GET", settingsPath), { default_workflow_permissions: "read", can_approve_pull_request_reviews: false }); | |
| 1026 | + assert.deepEqual(await ok("GET", settingsPath), { default_workflow_permissions: "read", can_approve_pull_request_reviews: false, agent_runs_use_secrets: false }); | |
| 1027 | 1027 | const bot = (await ok("POST", "/diffinery/agents", { login: "policy-bot" }, null)).token.token; |
| 1028 | 1028 | assert.equal((await api("PUT", settingsPath, { can_approve_pull_request_reviews: true }, bot)).status, 403, "agents cannot change what workflows may do"); |
| 1029 | 1029 | assert.equal((await api("PUT", settingsPath, { default_workflow_permissions: "admin" })).status, 422); |
| @@ -1039,9 +1039,9 @@ | ||
| 1039 | 1039 | assert.equal(r.status, 303); |
| 1040 | 1040 | }; |
| 1041 | 1041 | await form({ default_permissions: "write", can_approve: "1" }); |
| 1042 | - assert.deepEqual(await ok("GET", settingsPath), { default_workflow_permissions: "write", can_approve_pull_request_reviews: true }); | |
| 1042 | + assert.deepEqual(await ok("GET", settingsPath), { default_workflow_permissions: "write", can_approve_pull_request_reviews: true, agent_runs_use_secrets: false }); | |
| 1043 | 1043 | await form({ default_permissions: "read" }); |
| 1044 | - assert.deepEqual(await ok("GET", settingsPath), { default_workflow_permissions: "read", can_approve_pull_request_reviews: false }); | |
| 1044 | + assert.deepEqual(await ok("GET", settingsPath), { default_workflow_permissions: "read", can_approve_pull_request_reviews: false, agent_runs_use_secrets: false }); | |
| 1045 | 1045 | |
| 1046 | 1046 | const src = join(work, "policy"); |
| 1047 | 1047 | mkdirSync(join(src, ".github/workflows"), { recursive: true }); |
| @@ -1099,7 +1099,7 @@ | ||
| 1099 | 1099 | |
| 1100 | 1100 | // Allowed: the approval is recorded, but only people's approvals count toward the required ones. |
| 1101 | 1101 | await ok("PUT", settingsPath, { can_approve_pull_request_reviews: true }); |
| 1102 | - assert.deepEqual(await ok("GET", settingsPath), { default_workflow_permissions: "read", can_approve_pull_request_reviews: true }); | |
| 1102 | + assert.deepEqual(await ok("GET", settingsPath), { default_workflow_permissions: "read", can_approve_pull_request_reviews: true, agent_runs_use_secrets: false }); | |
| 1103 | 1103 | await ok("PUT", `/repos/${owner}/policy/branches/main/protection`, { |
| 1104 | 1104 | requirePullRequest: true, |
| 1105 | 1105 | requiredApprovals: 1, |
modifiedapps/diffinery/test/e2e/security.test.mjs+96−3View file
| @@ -135,6 +135,7 @@ | ||
| 135 | 135 | token("t10", "a-helper", T.helper, ["write"]), |
| 136 | 136 | `INSERT INTO org_members (org_id, user_id, role, created_at) VALUES ('o-shop', 'u-olive', 'owner', '${at}'), ('o-shop', 'u-sam', 'member', '${at}')`, |
| 137 | 137 | session("s4", "u-sam", S.sam), |
| 138 | + "UPDATE accounts SET email = 'sam@example.com' WHERE id = 'u-sam'", | |
| 138 | 139 | ].join("; "), |
| 139 | 140 | ); |
| 140 | 141 | dev = spawn( |
| @@ -618,14 +619,17 @@ | ||
| 618 | 619 | }); |
| 619 | 620 | |
| 620 | 621 | /** A web form post as olive (signed in by session), with the CSRF pair taken from `page`. */ |
| 621 | -async function oliveSubmits(page, action, fields) { | |
| 622 | - const cookie = `__Host-dfy_session=${S.olive}`; | |
| 622 | +const oliveSubmits = (page, action, fields) => submitsAs(S.olive, page, action, fields); | |
| 623 | + | |
| 624 | +/** A web form post signed in with `session`, with the CSRF pair taken from `page`. */ | |
| 625 | +async function submitsAs(session, page, action, fields) { | |
| 626 | + const cookie = `__Host-dfy_session=${session}`; | |
| 623 | 627 | const res = await fetch(`${base}${page}`, { headers: { cookie } }); |
| 624 | 628 | const csrf = /name="_csrf" value="([^"]+)"/.exec(await res.text())?.[1]; |
| 625 | 629 | const pair = /dfy_csrf=([^;]+)/.exec(res.headers.get("set-cookie") ?? "")?.[1]; |
| 626 | 630 | assert.ok(csrf && pair, `no CSRF pair on ${page} (${res.status})`); |
| 627 | 631 | const r = await fetch(`${base}${action}`, { method: "POST", body: new URLSearchParams({ _csrf: csrf, ...fields }), redirect: "manual", headers: { cookie: `${cookie}; dfy_csrf=${pair}`, "sec-fetch-site": "same-origin" } }); |
| 628 | - return { status: r.status, flash: decodeURIComponent(decodeURIComponent(r.headers.get("set-cookie") ?? "")) }; | |
| 632 | + return { status: r.status, flash: decodeURIComponent(decodeURIComponent(r.headers.get("set-cookie") ?? "")), text: await r.text() }; | |
| 629 | 633 | } |
| 630 | 634 | |
| 631 | 635 | /** Call an MCP tool with a token. */ |
| @@ -1650,3 +1654,92 @@ | ||
| 1650 | 1654 | g(["commit", "-q", "-m", "submodule"]); |
| 1651 | 1655 | assert.equal(push("HEAD").status, 0); |
| 1652 | 1656 | }); |
| 1657 | + | |
| 1658 | +test("security: a job's runner gets only the secrets the job names", async () => { | |
| 1659 | + const R = `/repos/${OWNER}/few-secrets`; | |
| 1660 | + await repoWithReadme("few-secrets"); | |
| 1661 | + await status(204, "PUT", `${R}/actions/secrets/USED`, { value: "used-value" }); | |
| 1662 | + await status(204, "PUT", `${R}/actions/secrets/UNUSED`, { value: "unused-value" }); | |
| 1663 | + const wf = "name: ci\non: push\njobs:\n build:\n runs-on: [self-hosted, linux]\n steps:\n - run: echo hi\n env:\n T: ${{ secrets.used }}\n dump:\n runs-on: [self-hosted, linux]\n steps:\n - run: echo '${{ toJSON(secrets) }}'\n"; | |
| 1664 | + await ok("PUT", `${R}/contents/.github/workflows/ci.yml`, { message: "ci", content: b64(wf) }); | |
| 1665 | + const specs = {}; | |
| 1666 | + await until("both jobs claimed", async () => { | |
| 1667 | + const r = await api("POST", `${R}/actions/runners/claim`, { name: "secrets-runner", labels: ["linux"] }); | |
| 1668 | + if (r.status === 200) { | |
| 1669 | + const spec = await (await fetch(`${base}/runner/jobs/${r.json.job_id}`, { headers: { authorization: `Bearer ${r.json.token}` } })).json(); | |
| 1670 | + specs[spec.jobKey] = Object.keys(spec.secrets).sort(); | |
| 1671 | + 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: {} }) }); | |
| 1672 | + } | |
| 1673 | + return Object.keys(specs).length === 2; | |
| 1674 | + }); | |
| 1675 | + assert.deepEqual(specs.build, ["DIFFINERY_TOKEN", "GITHUB_TOKEN", "USED"]); | |
| 1676 | + // toJSON(secrets) can't be narrowed, so that job gets them all, as on GitHub. | |
| 1677 | + assert.deepEqual(specs.dump, ["DIFFINERY_TOKEN", "GITHUB_TOKEN", "UNUSED", "USED"]); | |
| 1678 | +}); | |
| 1679 | + | |
| 1680 | +test("security: runs of agent code get no repository secrets unless an admin allows it", async () => { | |
| 1681 | + const R = `/repos/${OWNER}/agent-secrets`; | |
| 1682 | + await repoWithReadme("agent-secrets"); | |
| 1683 | + await status(204, "PUT", `${R}/collaborators/olive-bot`, { permission: "write" }); | |
| 1684 | + await status(204, "PUT", `${R}/actions/secrets/DEPLOY`, { value: "deploy-value" }); | |
| 1685 | + const settings = await ok("GET", `${R}/actions/permissions/workflow`); | |
| 1686 | + assert.equal(settings.agent_runs_use_secrets, false); | |
| 1687 | + const wf = (n) => `name: ci\non: push\njobs:\n build${n}:\n runs-on: [self-hosted, linux]\n steps:\n - run: echo hi\n env:\n T: \${{ secrets.DEPLOY }}\n`; | |
| 1688 | + const secretsOfNextJob = async () => { | |
| 1689 | + let names; | |
| 1690 | + await until("the agent's job", async () => { | |
| 1691 | + const r = await api("POST", `${R}/actions/runners/claim`, { name: "agent-runner", labels: ["linux"] }); | |
| 1692 | + if (r.status !== 200) return false; | |
| 1693 | + const spec = await (await fetch(`${base}/runner/jobs/${r.json.job_id}`, { headers: { authorization: `Bearer ${r.json.token}` } })).json(); | |
| 1694 | + names = Object.keys(spec.secrets).sort(); | |
| 1695 | + 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: {} }) }); | |
| 1696 | + return true; | |
| 1697 | + }); | |
| 1698 | + return names; | |
| 1699 | + }; | |
| 1700 | + // An agent pushes a workflow that asks for the deploy secret: its run gets only the job token. | |
| 1701 | + await ok("POST", `${R}/git/refs`, { ref: "refs/heads/agent-work", sha: (await ok("GET", `${R}/branches/main`)).commit.sha }, T.bot); | |
| 1702 | + await ok("PUT", `${R}/contents/.github/workflows/ci.yml`, { message: "ci", content: b64(wf(1)), branch: "agent-work" }, T.bot); | |
| 1703 | + assert.deepEqual(await secretsOfNextJob(), ["DIFFINERY_TOKEN", "GITHUB_TOKEN"]); | |
| 1704 | + // Only a person can allow it; then the agent's runs get what they name. | |
| 1705 | + await status(403, "PUT", `${R}/actions/permissions/workflow`, { agent_runs_use_secrets: true }, T.bot); | |
| 1706 | + await status(204, "PUT", `${R}/actions/permissions/workflow`, { agent_runs_use_secrets: true }); | |
| 1707 | + const file = await ok("GET", `${R}/contents/.github/workflows/ci.yml?ref=agent-work`, undefined, T.bot); | |
| 1708 | + await ok("PUT", `${R}/contents/.github/workflows/ci.yml`, { message: "ci 2", content: b64(wf(2)), branch: "agent-work", sha: file.sha }, T.bot); | |
| 1709 | + assert.deepEqual(await secretsOfNextJob(), ["DEPLOY", "DIFFINERY_TOKEN", "GITHUB_TOKEN"]); | |
| 1710 | +}); | |
| 1711 | + | |
| 1712 | +test("security: adding someone by email always invites them, so the address doesn't reveal their username and they agree first", async () => { | |
| 1713 | + await ok("POST", "/user/orgs", { login: "by-mail" }); | |
| 1714 | + const members = async () => (await ok("GET", "/orgs/by-mail/members")).map((u) => u.login); | |
| 1715 | + | |
| 1716 | + // sam has an account with this address, but the admin gets the same invitation as for a stranger. | |
| 1717 | + const added = await oliveSubmits("/by-mail?tab=people", "/by-mail/-/org/people", { who: "Sam@Example.com", role: "member" }); | |
| 1718 | + assert.equal(added.status, 200, added.flash); | |
| 1719 | + assert.match(added.text, /Invitation sent to Sam@Example\.com/); | |
| 1720 | + assert.doesNotMatch(added.flash + added.text, /Added sam|\bsam (is|now)\b/); | |
| 1721 | + assert.ok(!(await members()).includes("sam")); | |
| 1722 | + const link = /\/invite\/([A-Za-z0-9_-]+)/.exec(added.text)?.[1]; | |
| 1723 | + assert.ok(link); | |
| 1724 | + // The link the admin holds doesn't say whether the address has an account either. | |
| 1725 | + const landing = await (await fetch(`${base}/invite/${link}`)).text(); | |
| 1726 | + assert.match(landing, /Confirm your email/); | |
| 1727 | + assert.doesNotMatch(landing, /already has a Diffinery account/); | |
| 1728 | + | |
| 1729 | + // sam joins by accepting it. | |
| 1730 | + const accepted = await submitsAs(S.sam, `/invite/${link}`, `/invite/${link}`, {}); | |
| 1731 | + assert.equal(accepted.status, 303, accepted.flash); | |
| 1732 | + assert.ok((await members()).includes("sam")); | |
| 1733 | + | |
| 1734 | + // Inviting a member's address again doesn't name them. | |
| 1735 | + const again = await oliveSubmits("/by-mail?tab=people", "/by-mail/-/org/people", { who: "sam@example.com", role: "admin" }); | |
| 1736 | + assert.doesNotMatch(again.flash + again.text, /sam is already a member/); | |
| 1737 | + assert.match(again.text, /Invitation sent to sam@example\.com/); | |
| 1738 | + | |
| 1739 | + // The same for a repository's collaborators. | |
| 1740 | + await repoWithReadme("by-mail-repo"); | |
| 1741 | + const repo = await oliveSubmits(`/${OWNER}/by-mail-repo/settings/access`, `/${OWNER}/by-mail-repo/settings/access/people`, { who: "sam@example.com", role: "write" }); | |
| 1742 | + assert.equal(repo.status, 200, repo.flash); | |
| 1743 | + assert.match(repo.text, /Invitation sent to sam@example\.com/); | |
| 1744 | + await status(404, "GET", `/repos/${OWNER}/by-mail-repo`, undefined, T.sam); | |
| 1745 | +}); |
modifiedapps/diffinery/test/unit/actions-runtime.test.ts+2−2View file
| @@ -624,8 +624,8 @@ | ||
| 624 | 624 | // The web settings page calls it without `via`. |
| 625 | 625 | policy.setSettings({ canApprovePullRequests: true }, admin); |
| 626 | 626 | expect(events.map((e) => [e.action, e.detail])).toEqual([ |
| 627 | - ["repo.actions_workflow_settings", { defaultPermissions: "read", canApprovePullRequests: false, via: "api" }], | |
| 628 | - ["repo.actions_workflow_settings", { defaultPermissions: "read", canApprovePullRequests: true }], | |
| 627 | + ["repo.actions_workflow_settings", { defaultPermissions: "read", canApprovePullRequests: false, agentRunSecrets: false, via: "api" }], | |
| 628 | + ["repo.actions_workflow_settings", { defaultPermissions: "read", canApprovePullRequests: true, agentRunSecrets: false }], | |
| 629 | 629 | ]); |
| 630 | 630 | }); |
| 631 | 631 | }); |
addedapps/diffinery/test/unit/job-secrets.test.ts+28−0View file
| @@ -1,0 +1,28 @@ | ||
| 1 | +import { describe, expect, it } from "vitest"; | |
| 2 | +import { referencedSecrets, secretsFor } from "../../src/repo/job-secrets.js"; | |
| 3 | + | |
| 4 | +const job = (steps: unknown[], extra: Record<string, unknown> = {}) => ({ "runs-on": "linux", steps, ...extra }); | |
| 5 | + | |
| 6 | +describe("secrets a job references", () => { | |
| 7 | + it("finds names in expressions, with dot or index syntax, in any case", () => { | |
| 8 | + const used = referencedSecrets( | |
| 9 | + job([{ run: "deploy", env: { TOKEN: "${{ secrets.deploy_token }}" } }, { uses: "x/y@v1", with: { key: "${{ secrets['NPM_TOKEN'] }}" } }], { if: "secrets.FLAG != ''" }), | |
| 10 | + { GLOBAL: '${{ secrets["Global"] }}' }, | |
| 11 | + ); | |
| 12 | + expect(used).toEqual(new Set(["DEPLOY_TOKEN", "NPM_TOKEN", "FLAG", "GLOBAL"])); | |
| 13 | + }); | |
| 14 | + | |
| 15 | + it("ignores the word outside expressions and as a property name", () => { | |
| 16 | + expect(referencedSecrets(job([{ run: "echo no secrets here" }, { run: "${{ steps.secrets.outputs.x }}" }]))).toEqual(new Set()); | |
| 17 | + }); | |
| 18 | + | |
| 19 | + it("gives everything when an expression reads secrets as a whole", () => { | |
| 20 | + expect(referencedSecrets(job([{ run: "echo '${{ toJSON(secrets) }}'" }]))).toBe("all"); | |
| 21 | + expect(referencedSecrets(job([{ run: "${{ secrets[format('{0}_KEY', matrix.env)] }}" }]))).toBe("all"); | |
| 22 | + }); | |
| 23 | + | |
| 24 | + it("narrows the secrets map", () => { | |
| 25 | + expect(secretsFor({ A: "1", B: "2", Lower: "3" }, new Set(["A", "LOWER"]))).toEqual({ A: "1", Lower: "3" }); | |
| 26 | + expect(secretsFor({ A: "1" }, "all")).toEqual({ A: "1" }); | |
| 27 | + }); | |
| 28 | +}); |