Fork pull requests get no secrets; push protection reads large files; import limits (#50)
Jobs for pull requests from forks (imported ones included) get no secrets and a read-only token. Push protection scans text files of any size, through git and without it, and a commit blocked without git is emailed like a blocked push. Revoking a token through the API is audited. Imports refuse repositories over the size limit before copying anything, and organization variables limited to some repositories on GitHub aren't copied to every repository. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016yhgAu6oKfqes812Yd4qjo
Canberk Demirci authored · GitHub committed signed1 parent 7dd08e9commit acdea7601240c1178806dee60eabf3ad264d5d01Browse files
22 files changed+139−31
Files (22)
- apps/diffinery/docs/accounts/notifications.md+1−1
- apps/diffinery/docs/actions/secrets-and-variables.md+1−1
- apps/diffinery/docs/admin/site-administration.md+1−1
- apps/diffinery/docs/agents/mcp.md+1−1
- apps/diffinery/docs/reference/limits.md+1−1
- apps/diffinery/docs/repositories/git.md+2−2
- apps/diffinery/docs/security/push-protection.md+6−6
- apps/diffinery/docs/self-hosting/architecture.md+1−1
- apps/diffinery/src/http/api.ts+3−1
- apps/diffinery/src/imports/mapping.ts+3−0
- apps/diffinery/src/imports/service.ts+12−1
- apps/diffinery/src/mail/notify.ts+5−1
- apps/diffinery/src/mail/samples.ts+1−0
- apps/diffinery/src/mail/templates.ts+15−0
- apps/diffinery/src/mail/types.ts+1−1
- apps/diffinery/src/repo/RepoDO.ts+2−2
- apps/diffinery/src/repo/actions.ts+4−3
- apps/diffinery/src/repo/imports.ts+5−3
- apps/diffinery/src/repo/token-policy.ts+4−2
- apps/diffinery/test/e2e/e2e.test.mjs+31−2
- apps/diffinery/test/e2e/email.test.mjs+12−0
- apps/diffinery/test/e2e/security.test.mjs+27−1
modifiedapps/diffinery/docs/accounts/notifications.md+1−1View file
| @@ -233,7 +233,7 @@ | ||
| 233 | 233 | | `token_created` | `[Diffinery] A personal access token was added to your account` (or `A token was created for the agent review-bot`) | Security | `security_alert` | A token was created for you or your agent, from the web UI, CLI or API | |
| 234 | 234 | | `token_expiring` | `[Diffinery] Your token "ci-deploy" expires on <date>` | Security | `security_alert` | About 7 days before a token expires (once per token, for tokens that last more than 8 days) | |
| 235 | 235 | | `agent_created` | `[Diffinery] A new agent was added: review-bot` | Security | `security_alert` | You created an agent | |
| 236 | -| `push_blocked` | `[acme/website] Your push was blocked: possible secrets` | Security | `security_alert` | Push protection rejected your `git push` (for an agent's push, its owner is told). Commits refused in the web editor, the REST API or MCP are shown as an error and send no email. | | |
| 236 | +| `push_blocked` | `[acme/website] Your push was blocked: possible secrets` | Security | `security_alert` | Push protection rejected your `git push`, or a commit made in the web editor, through the REST API or over MCP (subject `Your commit was blocked`). For an agent, its owner is told. | | |
| 237 | 237 | | `push_bypassed` | `[acme/website] Push protection was bypassed` | Security | `security_alert` | A repository admin pushed past push protection, or committed a flagged file through the commits API with `allow_secrets`; sent to that admin and the repository's admins | |
| 238 | 238 | | `account_status` | `[Diffinery] Your account was disabled` or `Your account was enabled` | Security | `account` | A site administrator disabled your account, or enabled it again | |
| 239 | 239 | | `invite` | `@diffy invited you to join the acme organization on Diffinery` | Account | `invitation` | Someone invited your email address (organization, repository or instance) | |
modifiedapps/diffinery/docs/actions/secrets-and-variables.md+1−1View file
| @@ -285,7 +285,7 @@ | ||
| 285 | 285 | |
| 286 | 286 | Every workflow run in the repository receives its repository and organization secrets, whatever the event: `push`, `pull_request`, `pull_request_target`, `merge_group`, `workflow_dispatch`, `schedule`, `workflow_run`, `release` and `repository_dispatch`. Environment secrets follow stricter rules (see [Rules for environment secrets](#rules-for-environment-secrets)). |
| 287 | 287 | |
| 288 | -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. `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. | |
| 288 | +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 | 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 | 291 |
modifiedapps/diffinery/docs/admin/site-administration.md+1−1View file
| @@ -158,7 +158,7 @@ | ||
| 158 | 158 | | `POST /api/v3/diffinery/admin/emails/samples` | Send every template to yourself now, or only some with `{"only": ["signin", "invite-org"]}`. Returns `to`, `sent`, `total` and each result. | site administrator, with an `admin` token for all repositories or a browser session | |
| 159 | 159 | | `GET /api/v3/diffinery/emails?all=1` | Everyone's recent emails (without `all=1`, only yours). Also takes `kind`, `since` and `per_page` (up to 500). | site administrator for `all=1` | |
| 160 | 160 | |
| 161 | -The sample IDs are `pr-opened`, `issue-opened`, `comment`, `review-approved`, `review-changes`, `review-requested`, `assigned`, `pushed`, `merged`, `closed`, `reopened`, `run-failed`, `signin`, `passkey-added`, `passkey-removed`, `token-created`, `token-expiring`, `agent-created`, `push-bypassed`, `push-blocked`, `account-disabled`, `account-enabled`, `invite-org`, `invite-repo`, `invite-accepted`, `welcome`, `signin-link`, `access-request`, `org-member`, `repo-access`, `repo-created`, `repo-deleted`, `repo-transferred` and `test`. | |
| 161 | +The sample IDs are `pr-opened`, `issue-opened`, `comment`, `review-approved`, `review-changes`, `review-requested`, `assigned`, `pushed`, `merged`, `closed`, `reopened`, `run-failed`, `signin`, `passkey-added`, `passkey-removed`, `token-created`, `token-expiring`, `agent-created`, `push-bypassed`, `push-blocked`, `commit-blocked`, `account-disabled`, `account-enabled`, `invite-org`, `invite-repo`, `invite-accepted`, `welcome`, `signin-link`, `access-request`, `org-member`, `repo-access`, `repo-created`, `repo-deleted`, `repo-transferred` and `test`. | |
| 162 | 162 | |
| 163 | 163 | ## Administration from the CLI |
| 164 | 164 |
modifiedapps/diffinery/docs/agents/mcp.md+1−1View file
| @@ -241,7 +241,7 @@ | ||
| 241 | 241 | |
| 242 | 242 | It returns `{ "sha": "...", "branch": "..." }`. At least one file or delete is required (`nothing to commit`). Committing to a branch that doesn't exist without `create_branch_from` fails with `branch fix-pricing-link not found`. Commits made by agents are stamped with the agent and its owner; see [Agents](/docs/agents/overview#how-agent-activity-is-recorded). |
| 243 | 243 | |
| 244 | -`commit_files` runs [push protection](/docs/security/push-protection) before it writes anything, like `git push`. If a file under 1 MiB looks like a credential (an AWS key, a GitHub or Diffinery token, a private key and so on), nothing is committed and the tool fails with, for example, `push protection: possible secret detected (AWS access key) in config/deploy.env. Remove it, or rotate it and ask an admin.` The attempt is recorded in the audit log as `api.blocked_secret` against the token's account. `commit_files` has no bypass. For a test fixture with fake keys, put the text `diffinery:allow-secret` anywhere in the file. | |
| 244 | +`commit_files` runs [push protection](/docs/security/push-protection) before it writes anything, like `git push`. If a file looks like a credential (an AWS key, a GitHub or Diffinery token, a private key and so on), nothing is committed and the tool fails with, for example, `push protection: possible secret detected (AWS access key) in config/deploy.env. Remove it, or rotate it and ask an admin.` The attempt is recorded in the audit log as `api.blocked_secret` against the token's account, and the agent's owner gets a `Your commit was blocked` email. `commit_files` has no bypass. For a test fixture with fake keys, put the text `diffinery:allow-secret` anywhere in the file. | |
| 245 | 245 | |
| 246 | 246 | ## Releases over MCP |
| 247 | 247 |
modifiedapps/diffinery/docs/reference/limits.md+1−1View file
| @@ -122,7 +122,7 @@ | ||
| 122 | 122 | | --- | --- | --- | |
| 123 | 123 | | Push size | 100 MB per push | The whole push request after decompression. A larger push is rejected with `push exceeds 100 MB; split it into smaller pushes`. Push in several steps (for example a range of commits at a time). | |
| 124 | 124 | | Git protocol | Version 2 for fetch and clone | Clients older than git 2.26 must run `git config --global protocol.version 2`, or the server answers `Diffinery requires Git protocol v2`. | |
| 125 | -| Secret scanning coverage | New blobs smaller than 1 MiB | Push protection scans every new file under 1 MiB in a push. The rejection message lists up to 10 findings. | | |
| 125 | +| Secret scanning coverage | Every new text file | Push protection scans every new text file in a push, whatever its size. The rejection message lists up to 10 findings. | | |
| 126 | 126 | | Linear history check | 1,000 new commits | A protected branch with "require linear history" checks up to 1,000 new commits in a push for merge commits. | |
| 127 | 127 | |
| 128 | 128 | ## Git LFS |
modifiedapps/diffinery/docs/repositories/git.md+2−2View file
| @@ -218,7 +218,7 @@ | ||
| 218 | 218 | 1. **Authorization.** You must send a token: a push without credentials gets HTTP 401 (`authentication required`) with a `WWW-Authenticate` challenge, for public and private repositories alike, which makes Git ask for credentials and retry. Your account needs write access, your token the `write` or `admin` scope, and the repository must not be archived. Otherwise the server answers HTTP 403 (`write access required` when your role only allows reading, `this token needs the write scope` for a `read`-scoped token, or `repository is archived (read-only)`), or HTTP 404 if the account can't see the repository at all. Git shows these as `Authentication failed` or `The requested URL returned error: 403`. |
| 219 | 219 | 2. **Size check.** A push larger than 100 MB is refused. Git shows `remote unpack failed: push exceeds 100 MB; split it into smaller pushes`, or an HTTP error if the request is stopped before it reaches Diffinery. |
| 220 | 220 | 3. **Unpacking.** The pack is parsed and every object checked. |
| 221 | -4. **Push protection.** New text files under 1 MB are scanned for credentials such as AWS keys, GitHub and Diffinery tokens, Slack and Stripe keys, Google, Anthropic and OpenAI API keys, and private keys. If any are found, the whole push is rejected, nothing from it is stored, and the output lists each finding: | |
| 221 | +4. **Push protection.** New text files are scanned for credentials such as AWS keys, GitHub and Diffinery tokens, Slack and Stripe keys, Google, Anthropic and OpenAI API keys, and private keys. If any are found, the whole push is rejected, nothing from it is stored, and the output lists each finding: | |
| 222 | 222 | |
| 223 | 223 | ```text |
| 224 | 224 | remote: Push blocked: possible secrets detected. |
| @@ -310,7 +310,7 @@ | ||
| 310 | 310 | | Transport | HTTPS only. No SSH and no `git://`. | |
| 311 | 311 | | Fetch protocol | Protocol v2 only (Git 2.26+, or `protocol.version=2`). | |
| 312 | 312 | | Push size | 100 MB per push. Split bigger histories (see [Troubleshooting](#troubleshooting)). | |
| 313 | -| Large files | Use [Git LFS](/docs/repositories/git-lfs). Files over 1 MB in a push are not scanned by push protection. | | |
| 313 | +| Large files | Use [Git LFS](/docs/repositories/git-lfs). | | |
| 314 | 314 | | Partial clone, `--shallow-exclude` | Not supported yet. | |
| 315 | 315 | | `--deepen` | Behaves like `--depth`. | |
| 316 | 316 | | Packs sent to clients | Sent without deltas, so clones and fetches download more bytes than from GitHub. | |
modifiedapps/diffinery/docs/security/push-protection.md+6−6View file
| @@ -45,7 +45,7 @@ | ||
| 45 | 45 | | Scanned | Not scanned | |
| 46 | 46 | |---|---| |
| 47 | 47 | | Blobs (file contents) contained in the packfile of a `git push` | Commits, trees and tags themselves (commit messages are not scanned) | |
| 48 | -| In a push, text files smaller than 1 MiB (1,048,576 bytes) | In a push or a commit made without Git, files of 1 MiB or more | | |
| 48 | +| Text files of any size, in a push or a commit made without Git | | | |
| 49 | 49 | | Every ref in the push: branches and tags | Binary files: any blob that contains a NUL byte anywhere | |
| 50 | 50 | | Files you add or change in the web editor | Files that contain the text `diffinery:allow-secret` (see [Opting a file out](#opting-a-file-out)) | |
| 51 | 51 | | Files in a commit made with `POST /repos/{owner}/{repo}/diffinery/commits` or the MCP `commit_files` tool | Objects the server already has, which git does not send again | |
| @@ -55,7 +55,7 @@ | ||
| 55 | 55 | |
| 56 | 56 | Pushes larger than 100 MB are refused before scanning with `push exceeds 100 MB; split it into smaller pushes`. |
| 57 | 57 | |
| 58 | -> **Important:** Push protection is a safety net, not a guarantee. It finds only the formats listed above, in text files under 1 MiB. Never rely on it as your only defense: keep secrets in [Actions secrets](/docs/actions/secrets-and-variables) and out of your repository. | |
| 58 | +> **Important:** Push protection is a safety net, not a guarantee. It finds only the formats listed above, in text files. Never rely on it as your only defense: keep secrets in [Actions secrets](/docs/actions/secrets-and-variables) and out of your repository. | |
| 59 | 59 | |
| 60 | 60 | ## When a push is blocked |
| 61 | 61 | |
| @@ -127,7 +127,7 @@ | ||
| 127 | 127 | | `POST /repos/{owner}/{repo}/git/blobs` | The blob | |
| 128 | 128 | | `POST /repos/{owner}/{repo}/git/trees` | Each tree entry given with inline `content`; entries given by `sha` refer to blobs already stored and are not scanned again | |
| 129 | 129 | |
| 130 | -The patterns, the binary-file rule and the `diffinery:allow-secret` opt-out are the same as for pushes. Commits (the first four rows) skip files of 1 MiB or more, as pushes do. The git data endpoints (`git/blobs` and `git/trees`) scan every text file, whatever its size. | |
| 130 | +The patterns, the binary-file rule and the `diffinery:allow-secret` opt-out are the same as for pushes. Every text file is scanned, whatever its size. | |
| 131 | 131 | |
| 132 | 132 | ### When a commit is refused |
| 133 | 133 | |
| @@ -144,7 +144,7 @@ | ||
| 144 | 144 | |
| 145 | 145 | The git data endpoints answer `422` with the shorter `push protection: possible secret detected (AWS access key)`. |
| 146 | 146 | |
| 147 | -Each refused write is recorded in the [audit log](/docs/security/audit-log) as `api.blocked_secret`, against the person or agent who made the request, with the repository as the target and the detected types and paths (or, for the git data API, the blob id) as detail. No email is sent for it. | |
| 147 | +Each refused write is recorded in the [audit log](/docs/security/audit-log) as `api.blocked_secret`, against the person or agent who made the request, with the repository as the target and the detected types and paths (or, for the git data API, the blob id) as detail. You get an email with the subject `[acme/website] Your commit was blocked: possible secrets` (for an agent, its owner does). | |
| 148 | 148 | |
| 149 | 149 | ### Bypassing with the commits API |
| 150 | 150 | |
| @@ -213,7 +213,7 @@ | ||
| 213 | 213 | |
| 214 | 214 | ## Imported history |
| 215 | 215 | |
| 216 | -An [import from GitHub](/docs/migrate-from-github) copies a repository's Git history by pushing it into Diffinery in chunks, and push protection scans every one of those pushes like any other: each new blob under 1 MiB, with the same patterns and the same `diffinery:allow-secret` opt-out. Git LFS objects and release assets are not scanned, as for any push. Old history often holds credentials that were committed and removed later, so every import has a setting for what happens when push protection finds one: | |
| 216 | +An [import from GitHub](/docs/migrate-from-github) copies a repository's Git history by pushing it into Diffinery in chunks, and push protection scans every one of those pushes like any other: each new text blob, with the same patterns and the same `diffinery:allow-secret` opt-out. Git LFS objects and release assets are not scanned, as for any push. Old history often holds credentials that were committed and removed later, so every import has a setting for what happens when push protection finds one: | |
| 217 | 217 | |
| 218 | 218 | | Web form choice (under **Secrets found in old history (push protection)**) | API, CLI and MCP value | What happens | |
| 219 | 219 | |---|---|---| |
| @@ -265,7 +265,7 @@ | ||
| 265 | 265 | | The push is still blocked after you deleted the file in a new commit | The secret is still in an earlier commit you are pushing. Rewrite the commit that added it instead of adding a commit on top. | |
| 266 | 266 | | `Only repository admins (people, not agents) can bypass push protection.` | You used `-o allow-secrets=...` without being allowed. Check your role, that you are not using an agent's token, and that your token has the `admin` scope. | |
| 267 | 267 | | `-o allow-secrets=` seems to be ignored | The reason is empty. Give a non-empty reason, for example `-o allow-secrets="test fixture"`. | |
| 268 | -| A real key was not caught | It is not one of the formats above, it is in a binary file or in a pushed or committed file of 1 MiB or more, or the file contains `diffinery:allow-secret`. Rotate the key and remove it. | | |
| 268 | +| A real key was not caught | It is not one of the formats above, it is in a binary file, or the file contains `diffinery:allow-secret`. Rotate the key and remove it. | | |
| 269 | 269 | | `push protection: possible secret detected (...)` (422) from the API, MCP or the web editor | A file you committed or wrote matched. Remove the value, or mark a test fixture with `diffinery:allow-secret`. A repository admin who is a person can commit it anyway with `allow_secrets` on `POST /repos/{owner}/{repo}/diffinery/commits`; the other endpoints have no bypass. | |
| 270 | 270 | | `... Only repository admins (people, not agents) can bypass push protection.` (422) from the commits API | You sent `allow_secrets` without being allowed. Check your role, that you are not using an agent's token, and that your token has the `admin` scope. | |
| 271 | 271 | | An import from GitHub failed with `Push protection found possible secrets in the history (...)` | The source's history contains credentials and the import couldn't bypass. See [Imported history](#imported-history). | |
modifiedapps/diffinery/docs/self-hosting/architecture.md+1−1View file
| @@ -202,7 +202,7 @@ | ||
| 202 | 202 | 1. `git push` sends `POST /<owner>/<repo>.git/git-receive-pack` with a token in Basic authentication. |
| 203 | 203 | 2. The Worker resolves the token, loads the repository from D1, computes the caller's role (account role, capped by the token's scope and repository restriction, and by agent rules) and requires write access. A request without credentials gets `401 authentication required`, which makes Git ask for them, and a `read`-scoped token gets `403 this token needs the write scope`. It reads the body (decompressing gzip) and calls the repository's Durable Object. |
| 204 | 204 | 3. The Durable Object refuses pushes over 100 MB, parses the ref commands and the packfile, and resolves deltas against objects it already has. Nothing is stored yet. |
| 205 | -4. **Push protection** scans every new blob under 1 MiB for credentials. A finding blocks the whole push unless a human repository admin passed `-o allow-secrets=<reason>`. Both a block and a bypass are recorded as audit events. | |
| 205 | +4. **Push protection** scans every new text blob for credentials, whatever its size. A finding blocks the whole push unless a human repository admin passed `-o allow-secrets=<reason>`. Both a block and a bypass are recorded as audit events. | |
| 206 | 206 | 5. The objects are written (SQLite or R2 by size). |
| 207 | 207 | 6. Each ref update is checked: valid ref name, write role, a job token can't create, change or delete workflow files, `refs/pull/` and merge queue branches are read-only, objects exist, tags can only be moved by maintainers, agents can't push to the default or protected branches, and branch protection rules (pull request required, force push, deletion, linear history; admins bypass unless the rule is enforced for them). When several rules match the branch, the rule whose pattern is exactly the branch name applies; otherwise the oldest matching rule does. With `atomic`, one failure rejects all. |
| 208 | 208 | 7. Each accepted ref moves with a compare-and-swap, and the object records who moved it and whether that was a person or an agent. |
modifiedapps/diffinery/src/http/api.ts+3−1View file
| @@ -952,7 +952,9 @@ | ||
| 952 | 952 | }); |
| 953 | 953 | api.delete("/diffinery/tokens/:id", async (c) => { |
| 954 | 954 | requireAccountScope(c, "admin"); |
| 955 | - await c.get("dir").revokeToken(requireAccount(c).id, c.req.param("id")); | |
| 955 | + const a = requireAccount(c); | |
| 956 | + await c.get("dir").revokeToken(a.id, c.req.param("id")); | |
| 957 | + c.executionCtx.waitUntil(c.get("dir").audit({ actorId: a.id, actorLogin: a.login, action: "token.revoke", target: c.req.param("id"), detail: { via: "api" } })); | |
| 956 | 958 | return c.body(null, 204); |
| 957 | 959 | }); |
| 958 | 960 | api.get("/diffinery/agents", async (c) => { |
modifiedapps/diffinery/src/imports/mapping.ts+3−0View file
| @@ -165,6 +165,8 @@ | ||
| 165 | 165 | closedAt: string | null; |
| 166 | 166 | pull?: { |
| 167 | 167 | headRef: string; |
| 168 | + /** The GitHub repository a fork's head is in ("owner/name"); null for the same repository. */ | |
| 169 | + headRepo: string | null; | |
| 168 | 170 | headSha: string; |
| 169 | 171 | baseRef: string; |
| 170 | 172 | draft: boolean; |
| @@ -253,6 +255,7 @@ | ||
| 253 | 255 | stateReason: p.state === "closed" ? (merged ? "merged" : null) : null, |
| 254 | 256 | pull: { |
| 255 | 257 | headRef: sameRepo ? p.head.ref : `${forkOwner}:${p.head.ref}`, |
| 258 | + headRepo: sameRepo ? null : (p.head.repo?.full_name ?? `${forkOwner}/unknown`), | |
| 256 | 259 | headSha: p.head.sha, |
| 257 | 260 | baseRef: p.base.ref, |
| 258 | 261 | draft: !!p.draft, |
modifiedapps/diffinery/src/imports/service.ts+12−1View file
| @@ -740,6 +740,14 @@ | ||
| 740 | 740 | if (e instanceof GitHubError && e.status < 500) throw fatal(e.status === 401 ? "GitHub rejected the token (expired or revoked?)" : `GitHub repository ${ctx.row.source} was not found or the token cannot read it`); |
| 741 | 741 | throw e; |
| 742 | 742 | } |
| 743 | + // Each repository of an organization import is held to the same size limit as a single import. | |
| 744 | + if (!ctx.repo) { | |
| 745 | + try { | |
| 746 | + this.assertRepoSize(gr.size); | |
| 747 | + } catch (e) { | |
| 748 | + throw fatal((e as Error).message); | |
| 749 | + } | |
| 750 | + } | |
| 743 | 751 | const cloneUrl = gr.clone_url ?? `https://${webHostOf(ctx.row.api_url)}/${gr.full_name}.git`; |
| 744 | 752 | if (new URL(cloneUrl).host !== webHostOf(ctx.row.api_url)) throw fatal("GitHub returned a clone URL on another host"); |
| 745 | 753 | ctx.summary.source = { |
| @@ -1249,7 +1257,10 @@ | ||
| 1249 | 1257 | } |
| 1250 | 1258 | }; |
| 1251 | 1259 | try { |
| 1252 | - const vars = (await read<{ name: string; value: string; visibility?: string }>(`orgs/${ctx.row.source}/actions/variables?per_page=30`, "variables")).filter((v) => validVarName(v.name)); | |
| 1260 | + const listed = (await read<{ name: string; value: string; visibility?: string }>(`orgs/${ctx.row.source}/actions/variables?per_page=30`, "variables")).filter((v) => validVarName(v.name)); | |
| 1261 | + // Diffinery's organization variables reach every repository, so only GitHub's "all" ones are copied. | |
| 1262 | + const vars = listed.filter((v) => !v.visibility || v.visibility === "all"); | |
| 1263 | + for (const v of listed.filter((x) => !vars.includes(x))) await this.warn(id, `Organization variable ${v.name} is limited to ${v.visibility === "private" ? "private" : "selected"} repositories on GitHub and was not copied; organization variables on Diffinery reach every repository, so add it to the repositories that need it.`); | |
| 1253 | 1264 | const secrets = await read<{ name: string }>(`orgs/${ctx.row.source}/actions/secrets?per_page=100`, "secrets"); |
| 1254 | 1265 | // Add only variables the organization does not already have; never overwrite an admin's value. |
| 1255 | 1266 | const { added, skipped } = vars.length ? await this.dir.addOrgVariablesIfAbsent(owner.id, Object.fromEntries(vars.map((v) => [v.name, v.value]))) : { added: [], skipped: [] }; |
modifiedapps/diffinery/src/mail/notify.ts+5−1View file
| @@ -288,7 +288,8 @@ | ||
| 288 | 288 | break; |
| 289 | 289 | case "push.secret_bypassed": |
| 290 | 290 | case "api.secret_bypassed": |
| 291 | - case "push.blocked_secret": { | |
| 291 | + case "push.blocked_secret": | |
| 292 | + case "api.blocked_secret": { | |
| 292 | 293 | const [owner, name] = (e.target ?? "").split(":")[0]!.split("/"); |
| 293 | 294 | if (!owner || !name) return; |
| 294 | 295 | const repo = await dir.repo(owner, name); |
| @@ -296,7 +297,10 @@ | ||
| 296 | 297 | const kinds = Array.isArray(d.kinds) ? [...new Set(d.kinds as string[])] : []; |
| 297 | 298 | const refs = Array.isArray(d.refs) ? (d.refs as string[]) : undefined; |
| 298 | 299 | const pusher = await humansFor(dir, e.actorId); |
| 300 | + // A commit made without git (web editor, API, MCP) names the files instead of refs. | |
| 301 | + const paths = Array.isArray(d.paths) ? (d.paths as string[]) : []; | |
| 299 | 302 | if (e.action === "push.blocked_secret") jobs = to(pusher, { kind: "push_blocked", data: { repo: `${owner}/${name}`, kinds, refs } }); |
| 303 | + else if (e.action === "api.blocked_secret") jobs = to(pusher, { kind: "push_blocked", data: { repo: `${owner}/${name}`, kinds, paths } }); | |
| 300 | 304 | else jobs = to([...new Set([...pusher, ...(await repoAdmins(dir, repo))])], { kind: "push_bypassed", data: { repo: `${owner}/${name}`, pusher: e.actorLogin ?? "someone", kinds, reason: s("reason") ?? "", refs } }); |
| 301 | 305 | break; |
| 302 | 306 | } |
modifiedapps/diffinery/src/mail/samples.ts+1−0View file
| @@ -66,6 +66,7 @@ | ||
| 66 | 66 | item("agent-created", "Agent added", { kind: "agent_created", reason: "security_alert", data: { agent: "review-bot" } }), |
| 67 | 67 | item("push-bypassed", "Push protection bypassed", { kind: "push_bypassed", reason: "security_alert", data: { repo, pusher: me, kinds: ["AWS access key"], reason: "Fake key used by a test fixture", refs: ["refs/heads/main"] } }), |
| 68 | 68 | item("push-blocked", "Push blocked", { kind: "push_blocked", reason: "security_alert", data: { repo, kinds: ["GitHub token"], refs: ["refs/heads/feature"] } }), |
| 69 | + item("commit-blocked", "Commit blocked", { kind: "push_blocked", reason: "security_alert", data: { repo, kinds: ["AWS access key"], paths: ["config.env"] } }), | |
| 69 | 70 | item("account-disabled", "Account disabled", { kind: "account_status", reason: "account", data: { status: "disabled" } }), |
| 70 | 71 | item("account-enabled", "Account enabled", { kind: "account_status", reason: "account", data: { status: "active" } }), |
| 71 | 72 | item("invite-org", "Invitation (organization)", { kind: "invite", reason: "invitation", data: { inviter: me, url: `${pub}/invite/sample`, org: repo.split("/")[0], role: "member", expiresAt: exp } }), |
modifiedapps/diffinery/src/mail/templates.ts+15−0View file
| @@ -456,6 +456,21 @@ | ||
| 456 | 456 | } |
| 457 | 457 | case "push_blocked": { |
| 458 | 458 | const d = job.data; |
| 459 | + // A commit made without git (web editor, API or MCP) carries the files it was refused for. | |
| 460 | + if (d.paths) | |
| 461 | + return { | |
| 462 | + subject: `[${d.repo}] Your commit was blocked: possible secrets`, | |
| 463 | + preheader: `Found ${d.kinds.join(", ")}. Nothing was committed.`, | |
| 464 | + eyebrow: { text: "Push protection", tone: "red" }, | |
| 465 | + title: ["Your commit to ", { a: d.repo, href: `${pub}/${d.repo}` }, " was blocked"], | |
| 466 | + blocks: [ | |
| 467 | + { p: ["Push protection found what look like secrets in the files you tried to commit, so nothing was committed."] }, | |
| 468 | + { kv: [["Found", d.kinds.join(", ")], ...(d.paths.length ? ([["Files", d.paths.join(", ")]] as Array<[string, string]>) : []), ["When", at(job.at)]] }, | |
| 469 | + { p: ["Remove the secret and commit again. If it was a real credential, rotate it. For a test fixture, add a ", { code: "diffinery:allow-secret" }, " comment to the file."] }, | |
| 470 | + ], | |
| 471 | + context: repoContext(pub, d.repo), | |
| 472 | + url: `${pub}/${d.repo}`, | |
| 473 | + }; | |
| 459 | 474 | return { |
| 460 | 475 | subject: `[${d.repo}] Your push was blocked: possible secrets`, |
| 461 | 476 | preheader: `Found ${d.kinds.join(", ")}. Nothing from the push was kept.`, |
modifiedapps/diffinery/src/mail/types.ts+1−1View file
| @@ -77,7 +77,7 @@ | ||
| 77 | 77 | | { kind: "token_expiring"; data: { name: string; prefix: string; expiresAt: string; agent?: string } } |
| 78 | 78 | | { kind: "agent_created"; data: { agent: string } } |
| 79 | 79 | | { kind: "push_bypassed"; data: { repo: string; pusher: string; kinds: string[]; reason: string; refs?: string[] } } |
| 80 | - | { kind: "push_blocked"; data: { repo: string; kinds: string[]; refs?: string[] } } | |
| 80 | + | { kind: "push_blocked"; data: { repo: string; kinds: string[]; refs?: string[]; paths?: string[] } } | |
| 81 | 81 | | { kind: "account_status"; data: { status: "disabled" | "active" } } |
| 82 | 82 | | { kind: "invite"; data: { inviter: string; url: string; org?: string; repo?: string; role: string; expiresAt: string } } |
| 83 | 83 | | { kind: "invite_accepted"; data: { login: string; email: string; org?: string; repo?: string } } |
modifiedapps/diffinery/src/repo/RepoDO.ts+2−2View file
| @@ -299,7 +299,7 @@ | ||
| 299 | 299 | unpackError = (e as Error).message; |
| 300 | 300 | } |
| 301 | 301 | } |
| 302 | - const newBlobs = objects.filter((o) => o.type === "blob" && o.content.length < 1024 * 1024).map((o) => ({ oid: o.oid, content: o.content })); | |
| 302 | + const newBlobs = objects.filter((o) => o.type === "blob").map((o) => ({ oid: o.oid, content: o.content })); | |
| 303 | 303 | const results: RefResult[] = []; |
| 304 | 304 | if (unpackError) { |
| 305 | 305 | for (const c of req.commands) results.push({ ref: c.ref, ok: false, reason: "unpacker error" }); |
| @@ -562,7 +562,7 @@ | ||
| 562 | 562 | * web editor, the contents and commits APIs and MCP cannot be used to slip a credential in. |
| 563 | 563 | */ |
| 564 | 564 | private protectCommit(changes: readonly FileChange[], allowSecrets: string | undefined, caller: Caller): void { |
| 565 | - const blobs = changes.filter((ch) => !ch.delete && ch.content && ch.content.length < 1024 * 1024).map((ch) => ({ oid: ch.path, content: ch.content! })); | |
| 565 | + const blobs = changes.filter((ch) => !ch.delete && ch.content).map((ch) => ({ oid: ch.path, content: ch.content! })); | |
| 566 | 566 | const leaks = scanForSecrets(blobs); |
| 567 | 567 | if (!leaks.length) return; |
| 568 | 568 | const kinds = [...new Set(leaks.map((l) => l.kind))]; |
modifiedapps/diffinery/src/repo/actions.ts+4−3View file
| @@ -1167,7 +1167,7 @@ | ||
| 1167 | 1167 | |
| 1168 | 1168 | /** |
| 1169 | 1169 | * A pull_request run for a pull request from another repository (a fork): its jobs |
| 1170 | - * never get write. Decided from the pull request's row, not the event payload. | |
| 1170 | + * never get write or secrets. Decided from the pull request's row, not the event payload. | |
| 1171 | 1171 | */ |
| 1172 | 1172 | private untrusted(run: RunRow): boolean { |
| 1173 | 1173 | return run.event === "pull_request" && this.repo.policy.isForkPull(run.pr_number); |
| @@ -1183,7 +1183,8 @@ | ||
| 1183 | 1183 | const short = localKey(job.job_key, parent); |
| 1184 | 1184 | let scope: { inputs: Record<string, unknown>; secrets: Record<string, string> }; |
| 1185 | 1185 | try { |
| 1186 | - scope = await this.callContext(run, exp, parent, true); | |
| 1186 | + // A fork's code never sees the repository's secrets, as on GitHub. | |
| 1187 | + scope = await this.callContext(run, exp, parent, !this.untrusted(run)); | |
| 1187 | 1188 | } catch (e) { |
| 1188 | 1189 | await this.completeJob(jobId, { conclusion: "failure", outputs: {}, failure: { message: (e as Error).message } }); |
| 1189 | 1190 | throw new HttpError(409, (e as Error).message); |
| @@ -1209,7 +1210,7 @@ | ||
| 1209 | 1210 | this.sql.exec("UPDATE jobs SET status = 'in_progress', started_at = ? WHERE id = ?", t, jobId); |
| 1210 | 1211 | 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}` }); |
| 1211 | 1212 | this.repo.broadcast(`run:${run.id}`, { action: "job", jobId }); |
| 1212 | - const secrets: Record<string, string> = { ...scope.secrets, ...environment.secrets, GITHUB_TOKEN: token, DIFFINERY_TOKEN: token }; | |
| 1213 | + const secrets: Record<string, string> = { ...scope.secrets, ...(this.untrusted(run) ? {} : environment.secrets), GITHUB_TOKEN: token, DIFFINERY_TOKEN: token }; | |
| 1213 | 1214 | // The job token can do what the effective `permissions:` allow (job level wins, like GitHub). |
| 1214 | 1215 | const permissions = permissionsForJob(exp, job.job_key, { untrusted: this.untrusted(run), repoDefault: this.repo.policy.settings().defaultPermissions }); |
| 1215 | 1216 | this.repo.runtime.grant(jobId, permissions); |
modifiedapps/diffinery/src/repo/imports.ts+5−3View file
| @@ -113,14 +113,16 @@ | ||
| 113 | 113 | if (i.pull) { |
| 114 | 114 | const p = i.pull; |
| 115 | 115 | this.sql.exec( |
| 116 | - `INSERT INTO pulls (number, head_ref, head_sha, base_ref, draft, merged, merged_at, merged_by, merge_commit, requested_reviewers, imported) | |
| 117 | - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) | |
| 118 | - ON CONFLICT(number) DO UPDATE SET head_ref = excluded.head_ref, head_sha = excluded.head_sha, base_ref = excluded.base_ref, draft = excluded.draft, | |
| 116 | + `INSERT INTO pulls (number, head_ref, head_repo_id, head_sha, base_ref, draft, merged, merged_at, merged_by, merge_commit, requested_reviewers, imported) | |
| 117 | + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) | |
| 118 | + ON CONFLICT(number) DO UPDATE SET head_ref = excluded.head_ref, head_repo_id = excluded.head_repo_id, head_sha = excluded.head_sha, base_ref = excluded.base_ref, draft = excluded.draft, | |
| 119 | 119 | merged = excluded.merged, merged_at = excluded.merged_at, merged_by = COALESCE(excluded.merged_by, pulls.merged_by), |
| 120 | 120 | merge_commit = excluded.merge_commit, requested_reviewers = excluded.requested_reviewers, imported = excluded.imported |
| 121 | 121 | WHERE pulls.imported = excluded.imported`, |
| 122 | 122 | i.number, |
| 123 | 123 | p.headRef, |
| 124 | + // A fork on GitHub: marks the pull request as one whose runs get no write and no secrets. | |
| 125 | + p.headRepo ? `github:${p.headRepo}` : null, | |
| 124 | 126 | p.headSha, |
| 125 | 127 | p.baseRef, |
| 126 | 128 | p.draft ? 1 : 0, |
modifiedapps/diffinery/src/repo/token-policy.ts+4−2View file
| @@ -99,8 +99,10 @@ | ||
| 99 | 99 | /** A pull request whose head is in another repository (a fork), from the pull request's own row. */ |
| 100 | 100 | isForkPull(number: number | null): boolean { |
| 101 | 101 | if (number === null) return false; |
| 102 | - const r = this.sql.exec<{ head_repo_id: string | null }>("SELECT head_repo_id FROM pulls WHERE number = ?", number).toArray()[0]; | |
| 103 | - return !!r?.head_repo_id && r.head_repo_id !== this.repo.meta.repoId; | |
| 102 | + const r = this.sql.exec<{ head_repo_id: string | null; head_ref: string }>("SELECT head_repo_id, head_ref FROM pulls WHERE number = ?", number).toArray()[0]; | |
| 103 | + if (!r) return false; | |
| 104 | + // Imported pull requests from GitHub forks keep their head as "owner:branch", never a local branch. | |
| 105 | + return (!!r.head_repo_id && r.head_repo_id !== this.repo.meta.repoId) || r.head_ref.includes(":"); | |
| 104 | 106 | } |
| 105 | 107 | |
| 106 | 108 | // ---------------------------------------------------------------- workflow files |
modifiedapps/diffinery/test/e2e/e2e.test.mjs+31−2View file
| @@ -2239,6 +2239,7 @@ | ||
| 2239 | 2239 | const bob = u("bob", 3001); |
| 2240 | 2240 | let ghBase; |
| 2241 | 2241 | let linkNonce = ""; // set from the link challenge, then served back in octo-dev's public gist |
| 2242 | + let listHuge = false; // the organization lists an oversized repository once set | |
| 2242 | 2243 | const pr = (number, extra) => ({ number, title: `PR ${number}`, body: `Body of #${number}`, state: "open", user: alice, labels: [], assignees: [], milestone: null, locked: false, created_at: t, updated_at: t, closed_at: null, draft: false, merged_at: null, merge_commit_sha: null, requested_reviewers: [], head: { ref: "x", sha: fx.mainSha, label: "octo:x", repo: { full_name: "octo/app" } }, base: { ref: "main", repo: { full_name: "octo/app" } }, ...extra }); |
| 2243 | 2244 | const issue = (number, extra) => ({ number, title: `Issue ${number}`, body: `Body of #${number}`, state: "open", state_reason: null, user: octoDev, labels: [], assignees: [], milestone: null, locked: false, created_at: t, updated_at: t, closed_at: null, ...extra }); |
| 2244 | 2245 | const pulls = [ |
| @@ -2262,8 +2263,10 @@ | ||
| 2262 | 2263 | "GET /users/octo-dev/gists": () => [{ id: "proofgist", public: true, description: "", files: { "proof.md": { raw_url: `${ghBase}/raw/proofgist` } } }], |
| 2263 | 2264 | "GET /gists/proofgist": () => ({ files: { "proof.md": { content: `linking to diffinery: ${linkNonce}` } } }), |
| 2264 | 2265 | "GET /users/octo": { login: "octo", type: "Organization" }, |
| 2265 | - "GET /orgs/octo/repos": () => [repoJson("app"), repoJson("lib", { default_branch: "trunk", topics: [] })].map((r) => ({ ...r, clone_url: `${ghBase}/${r.full_name}.git` })), | |
| 2266 | - "GET /orgs/octo/actions/variables": { variables: [{ name: "ORG_REGION", value: "eu-west" }] }, | |
| 2266 | + "GET /orgs/octo/repos": () => [repoJson("app"), repoJson("lib", { default_branch: "trunk", topics: [] }), ...(listHuge ? [repoJson("huge", { size: 99_999_999 })] : [])].map((r) => ({ ...r, clone_url: `${ghBase}/${r.full_name}.git` })), | |
| 2267 | + "GET /repos/octo/huge": () => ({ ...repoJson("huge", { size: 99_999_999 }), clone_url: `${ghBase}/octo/huge.git` }), | |
| 2268 | + // Only ORG_REGION reaches every repository on GitHub; Diffinery's organization variables always do. | |
| 2269 | + "GET /orgs/octo/actions/variables": { variables: [{ name: "ORG_REGION", value: "eu-west", visibility: "all" }, { name: "ORG_PRIVATE_HOST", value: "internal.example", visibility: "private" }, { name: "ORG_SELECTED_KEY", value: "few", visibility: "selected" }] }, | |
| 2267 | 2270 | "GET /orgs/octo/actions/secrets": { secrets: [{ name: "ORG_DEPLOY_KEY" }] }, |
| 2268 | 2271 | "GET /repos/octo/app": () => ({ ...repoJson("app"), clone_url: `${ghBase}/octo/app.git`, html_url: `${ghBase}/octo/app` }), |
| 2269 | 2272 | "GET /repos/octo/app/labels": [{ name: "bug", color: "d73a4a", description: "Something isn't working" }, { name: "docs", color: "0075ca", description: "Documentation" }, { name: "wontfix", color: "ffffff", description: "" }], |
| @@ -2467,6 +2470,20 @@ | ||
| 2467 | 2470 | assert.equal(p6.head.ref, "forker:patch"); |
| 2468 | 2471 | assert.equal(p6.head.sha, fx.forkSha); |
| 2469 | 2472 | assert.equal(p6.merged, false); |
| 2473 | + // A pull request from a fork runs the fork's code, so its jobs get what GitHub gives them: | |
| 2474 | + // a read-only token and no secrets. | |
| 2475 | + assert.equal((await api("PUT", `/repos/${owner}/imported-app/actions/secrets/DEPLOY_KEY`, { value: "never-for-forks" })).status, 204); | |
| 2476 | + await ok("PATCH", `/repos/${owner}/imported-app/pulls/6`, { state: "open" }); | |
| 2477 | + const forkJob = await until("the fork pull request's job", async () => { | |
| 2478 | + const r = await api("POST", `/repos/${owner}/imported-app/actions/runners/claim`, { name: "fork-runner", labels: ["linux"] }); | |
| 2479 | + return r.status === 200 ? r.json : null; | |
| 2480 | + }); | |
| 2481 | + const forkSpec = await (await fetch(`${base}/runner/jobs/${forkJob.job_id}`, { headers: { authorization: `Bearer ${forkJob.token}` } })).json(); | |
| 2482 | + assert.equal(forkSpec.github.event_name, "pull_request"); | |
| 2483 | + assert.deepEqual(Object.keys(forkSpec.secrets).sort(), ["DIFFINERY_TOKEN", "GITHUB_TOKEN"]); | |
| 2484 | + assert.equal((await api("POST", `/repos/${owner}/imported-app/issues`, { title: "from a fork's job" }, forkJob.token)).status, 403); | |
| 2485 | + await fetch(`${base}/runner/jobs/${forkJob.job_id}/complete`, { method: "POST", headers: { authorization: `Bearer ${forkJob.token}`, "content-type": "application/json" }, body: JSON.stringify({ conclusion: "success", outputs: {} }) }); | |
| 2486 | + await ok("PATCH", `/repos/${owner}/imported-app/pulls/6`, { state: "closed" }); | |
| 2470 | 2487 | // New issues continue after the highest imported number. |
| 2471 | 2488 | assert.equal((await ok("POST", `/repos/${owner}/imported-app/issues`, { title: "After import" })).number, 8); |
| 2472 | 2489 | // Labels, milestones, protection, variables. |
| @@ -2505,6 +2522,7 @@ | ||
| 2505 | 2522 | assert.deepEqual(bulkDone.summary.orgSecretNames, ["ORG_DEPLOY_KEY"]); |
| 2506 | 2523 | const orgVars = await ok("GET", "/orgs/acme-imports/actions/variables"); |
| 2507 | 2524 | assert.deepEqual(orgVars.variables.map((v) => [v.name, v.value]), [["ORG_REGION", "eu-west"]]); |
| 2525 | + assert.ok(bulkDone.warnings.some((w) => /ORG_PRIVATE_HOST/.test(w)) && bulkDone.warnings.some((w) => /ORG_SELECTED_KEY/.test(w)), JSON.stringify(bulkDone.warnings)); | |
| 2508 | 2526 | assert.equal((await ok("GET", "/repos/acme-imports/lib")).default_branch, "trunk"); |
| 2509 | 2527 | assert.equal((await ok("GET", "/repos/acme-imports/lib/issues/1")).title, "Lib issue"); |
| 2510 | 2528 | assert.equal((await ok("GET", "/repos/acme-imports/app/pulls/2")).merged, true); |
| @@ -2543,6 +2561,17 @@ | ||
| 2543 | 2561 | const stalePush = gitFails(copy, ["push", `${base}/${owner}/imported-app.git`, "--push-option=diffinery-import", "refs/heads/main:refs/heads/stale-attempt"], agent.pushToken); |
| 2544 | 2562 | assert.match(stalePush, /401|403|denied|forbidden|authentication|fatal/i, stalePush); |
| 2545 | 2563 | |
| 2564 | + // ------------------------------------------------------------ an organization import holds each repository to the size limit | |
| 2565 | + listHuge = true; | |
| 2566 | + const hugeBulk = await ok("POST", "/diffinery/imports", { source_owner: "octo", repositories: ["huge"], owner: "acme-imports", github_token: GH_TOKEN, github_api_url: apiUrl, concurrency: 1, options: { lfs: false } }); | |
| 2567 | + const hugeChild = await until("the oversized repository to be refused", async () => { | |
| 2568 | + const ch = (await ok("GET", `/diffinery/imports/${hugeBulk.id}`)).children?.find((c) => c.source === "octo/huge"); | |
| 2569 | + return ch?.status === "failed" ? ch : ch?.git?.phase === "waiting" ? Promise.reject(new Error("the oversized repository was queued for transfer")) : null; | |
| 2570 | + }, 120_000); | |
| 2571 | + assert.match(hugeChild.error ?? (await ok("GET", `/diffinery/imports/${hugeChild.id}`)).error, /import limit/); | |
| 2572 | + assert.equal((await api("GET", "/repos/acme-imports/huge")).status, 404); | |
| 2573 | + await finished(hugeBulk.id); | |
| 2574 | + | |
| 2546 | 2575 | // ------------------------------------------------------------ the web UI (signed in via DEV_AUTH) |
| 2547 | 2576 | for (const path of ["/new/import", "/imports", `/imports/${started.id}`, `/imports/${started.id}/secrets`, `/imports/${bulk.id}`, "/settings/linked-accounts", `/${owner}/imported-app/issues/1`, `/${owner}/imported-app/pull/2`]) { |
| 2548 | 2577 | const res = await fetch(`${base}${path}`); |
modifiedapps/diffinery/test/e2e/email.test.mjs+12−0View file
| @@ -255,3 +255,15 @@ | ||
| 255 | 255 | assert.equal(failed.dev_copy.headers["X-Diffinery-Reason"], "ci_activity"); |
| 256 | 256 | assert.match(failed.dev_copy.text, /test/); |
| 257 | 257 | }); |
| 258 | + | |
| 259 | +test("email: a commit blocked by push protection, made without git, is emailed like a blocked push", async () => { | |
| 260 | + await ok("POST", "/user/repos", { name: "leak-mail", auto_init: true }); | |
| 261 | + // Built at run time so this file holds no credential-shaped string. | |
| 262 | + const leak = `AWS_ACCESS_KEY_ID=${"AKIA"}IOSFODNN7EXAMPL3\n`; | |
| 263 | + const r = await api("PUT", `/repos/${me.login}/leak-mail/contents/config.env`, { message: "leak", content: Buffer.from(leak).toString("base64") }); | |
| 264 | + assert.equal(r.status, 422, JSON.stringify(r.json)); | |
| 265 | + const blocked = await email("blocked commit", (e) => e.kind === "push_blocked" && /leak-mail/.test(e.subject)); | |
| 266 | + assert.equal(blocked.to_addr, me.email); | |
| 267 | + assert.match(blocked.dev_copy.text, /config\.env/); | |
| 268 | + assert.match(blocked.dev_copy.text, /AWS access key/); | |
| 269 | +}); |
modifiedapps/diffinery/test/e2e/security.test.mjs+27−1View file
| @@ -22,7 +22,7 @@ | ||
| 22 | 22 | // merge queue) where a person allowed it and a required check gates it, never by bypass. |
| 23 | 23 | import { spawn, spawnSync, execFileSync } from "node:child_process"; |
| 24 | 24 | import { createHash, randomBytes } from "node:crypto"; |
| 25 | -import { mkdtempSync, rmSync } from "node:fs"; | |
| 25 | +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; | |
| 26 | 26 | import { tmpdir } from "node:os"; |
| 27 | 27 | import { join, dirname, resolve } from "node:path"; |
| 28 | 28 | import { fileURLToPath } from "node:url"; |
| @@ -1409,3 +1409,29 @@ | ||
| 1409 | 1409 | // Removing it. |
| 1410 | 1410 | assert.equal((await ok("PATCH", `${R}/issues/${planned.number}`, { milestone: null })).milestone, null); |
| 1411 | 1411 | }); |
| 1412 | + | |
| 1413 | +test("security: push protection reads large files, and revoking a token through the API is on the record", async () => { | |
| 1414 | + const R = `/repos/${OWNER}/big-leak`; | |
| 1415 | + const mainSha = await repoWithReadme("big-leak"); | |
| 1416 | + // A credential after 1.5 MiB of text, built at run time so this file holds none. | |
| 1417 | + const big = `${"x".repeat(80) + "\n"}`.repeat(20_000) + `AWS_ACCESS_KEY_ID=${"AKIA"}IOSFODNN7EXAMPL3\n`; | |
| 1418 | + assert.ok(big.length > 1.5 * 1024 * 1024); | |
| 1419 | + const viaContents = await status(422, "PUT", `${R}/contents/big.env`, { message: "leak", content: b64(big) }); | |
| 1420 | + assert.match(viaContents.message, /push protection.*AWS access key/); | |
| 1421 | + await status(422, "POST", `${R}/diffinery/commits`, { branch: "main", message: "leak", files: [{ path: "big.env", content: big }] }); | |
| 1422 | + // ...and the same file through git. | |
| 1423 | + const clone = join(work, "big-leak"); | |
| 1424 | + const authed = `${base.replace("://", `://x:${T.olive}@`)}/${OWNER}/big-leak.git`; | |
| 1425 | + execFileSync("git", ["clone", "-q", authed, clone], { stdio: "pipe" }); | |
| 1426 | + writeFileSync(join(clone, "big.env"), big); | |
| 1427 | + const g = (...args) => execFileSync("git", ["-c", "user.name=Olive", "-c", "user.email=olive@example.com", ...args], { cwd: clone, stdio: "pipe" }); | |
| 1428 | + g("add", "big.env"); | |
| 1429 | + g("commit", "-q", "-m", "leak"); | |
| 1430 | + assert.throws(() => g("push", "-q", "origin", "HEAD:main"), /secret detected/); | |
| 1431 | + assert.equal((await ok("GET", `${R}/branches/main`)).commit.sha, mainSha); | |
| 1432 | + | |
| 1433 | + // ------------------------------------------------------------ revoking a token through the API is audited, like the settings page | |
| 1434 | + const minted = await ok("POST", "/diffinery/tokens", { name: "short-lived", scopes: ["read"] }, { cookie: S.olive }); | |
| 1435 | + await status(204, "DELETE", `/diffinery/tokens/${minted.id}`); | |
| 1436 | + await until("the revoke in the audit log", async () => auditLog("token.revoke").some((e) => e.target === minted.id && JSON.parse(e.detail ?? "{}").via === "api")); | |
| 1437 | +}); |