diff --git a/docs/agents.md b/docs/agents.md index 129e496..bd4a538 100644 --- a/docs/agents.md +++ b/docs/agents.md @@ -92,7 +92,9 @@ configures it: `hub.yml`, `targets.yml`, `.touchmark/operations.yml`, the CI fil * **Accounts.** A **reader** (read-only) runs `plan` on hub pull requests. A **writer**, a different account, runs `distribute` and `doctor` from the hub's default branch only. The **hub channel**, the CI's own token (`GITHUB_TOKEN`, `CI_JOB_TOKEN`), reaches the hub - repository alone. The writer has no access to the hub. + repository alone. The writer has no access to the hub, unless the hub says + `security.writer_on_hub: guard` (GitLab: the writer reaches the hub as a Developer, and + touchmark verifies it cannot get content onto the default branch). * **Fingerprint.** The hub is known by its host and immutable repository id (`github.com/712345678`), which CI provides. `id` in `hub.yml` names the sync branch `touchmark/` and is for people. @@ -262,6 +264,7 @@ ten `::error` and ten `::warning` annotations. | `memory.auto_close_cooldown` | `30d` | how long content a bot closed waits before it is proposed again | | `security.write_isolation` | `platform` | `platform` (CI keeps the key to the default branch; probed), `external` (an OIDC secret store), `none` (needs `security.reason`) | | `security.private_targets_in_public_hub` | `skip` | `deliver` puts their names into public CI logs | +| `security.writer_on_hub` | `refuse` | `guard` (GitLab): the writer may reach the hub as a Developer; `doctor` verifies the guard (`hub-guard`), `plan` refuses a hub merge request the writer opened or pushed to | | `sensitive_paths` | none | patterns added to the built-in list highlighted under ⚠ in pull requests | | `packs..description`, `.requires`, `.formerly` | none | metadata; `requires` adds and orders dependencies; `formerly` keeps a renamed pack's history | @@ -324,7 +327,12 @@ from its environment once read: probe does not test. 5. **The writer has no access to the hub.** Install the Apps on targets only; keep the hub outside the GitLab groups the accounts belong to. `doctor` fails when the writer - can push to the hub, and warns when it can see a private one. + can push to the hub, and warns when it can see a private one. On GitLab a hub that + must live in the group of its targets sets `security.writer_on_hub: guard`: the writer + is then a Developer of the hub, `doctor`'s `hub-guard` verifies that it can neither + push nor merge to the default branch and that merge request pipelines run the default + branch's CI file and must succeed, and `plan` exits 2 in a hub merge request the + writer opened or pushed to. 6. **Nothing reaches a repository without its consent.** `.engineering-assets.yml` at the root, possibly empty, or an entry with `opt_in: assumed` when the hub owns the decision; `enabled: false` in the file overrides the hub. Never ship that file in a diff --git a/docs/concepts/security.md b/docs/concepts/security.md index fa61f35..6bd5baa 100644 --- a/docs/concepts/security.md +++ b/docs/concepts/security.md @@ -23,10 +23,36 @@ closed where it is missing. separate account has its own rate limits. - **The writer has no access to the hub**, so a leaked write key cannot change the packs. `doctor` fails when the writer can push to the hub, and warns when it can see a - private one. + private one. See [A writer on the hub](#a-writer-on-the-hub-gitlab) for a GitLab hub + that has to live in the group of its targets. - **`plan` holds no write credential**, and a test of the code's call graph keeps it from ever reaching one. +## A writer on the hub (GitLab) + +On GitLab a member of a group inherits its projects, and GitLab cannot lower an inherited +role on one project. A hub kept in the group that holds its targets is therefore reached +by the writer, a group access token or a service account of that group. What rule 5 +protects is that a leaked write key cannot change what `distribute` ships; +`security.writer_on_hub: guard` gets that by verifying the hub instead of hiding it: + +- the writer is below Maintainer on the hub (a Maintainer changes its protection, + settings and variables); +- the default branch is protected, and no rule that matches it lets the writer push or + merge (GitLab applies the most permissive matching rule); +- merge request pipelines run the CI file of the default branch + (*CI/CD configuration file* `.gitlab-ci.yml@:`): otherwise the + writer, pushing to a merge request's branch, could rewrite the CI file to drop the plan; +- a merge waits for a pipeline that succeeded, and a skipped pipeline does not count; +- `plan`, in every hub merge request pipeline, exits 2 when the writer or one of its + `known_authors` opened the merge request or pushed to its source branch since the + branch was created (read from GitLab's push events, which name who pushed), and when it + cannot tell; the setting comes from the default branch's `hub.yml`. + +`doctor` grades these as `hub-guard` instead of `hub-hidden`, `doctor --hub-token` grades +the settings with a maintainer's token, and `touchmark setup gitlab` sets them up. Other +platforms report `hub-guard` as unknown: keep the writer off the hub there. + ## The write key stays on the default branch The write key must be visible only to jobs of the hub's default branch, which only diff --git a/docs/getting-started/gitlab.md b/docs/getting-started/gitlab.md index 7f8ab75..f0d45bf 100644 --- a/docs/getting-started/gitlab.md +++ b/docs/getting-started/gitlab.md @@ -19,7 +19,11 @@ See [Set up a hub's platform](../guide/setup.md#gitlab). to, and add the accounts, or create the group access tokens, only on groups that hold targets and not the hub. A member of a group inherits its projects, and a group access token is a member of its group, so a writer on a group that holds the hub could push - to it; `touchmark setup gitlab` refuses that layout, and `doctor` fails on it. + to it; `touchmark setup gitlab` refuses that layout, and `doctor` fails on it. If the + hub must stay in that group, set `security.writer_on_hub: guard` in `hub.yml` first: + setup then accepts the layout and sets up what keeps the writer, a Developer of the + hub, from getting content onto its default branch (see + [A writer on the hub](../concepts/security.md#a-writer-on-the-hub-gitlab)). - The *reader* has the role Reporter and a token with `read_api` and `read_repository`. Store it as the variable `TOUCHMARK_READ_TOKEN`, masked but not protected: merge request pipelines need it. diff --git a/docs/guide/doctor.md b/docs/guide/doctor.md index 1295284..70d81fd 100644 --- a/docs/guide/doctor.md +++ b/docs/guide/doctor.md @@ -20,6 +20,7 @@ Each check has a name, a status and a detail. The names you will see: | `scopes`, `2fa` | the write identity (Gitea, Forgejo) | the token lacks a scope the writer needs, or has more than it needs | | `signing-key` | the write identity | `TOUCHMARK__SIGNING_KEY` is set but cannot sign for the writer | | `hub-hidden` | the write identity | the writer can see the hub, so a leaked write key could change the packs | +| `hub-guard` | the write identity, under `security.writer_on_hub: guard` | the writer reaches the hub and could get content onto its default branch: a Maintainer role, a rule that lets it push or merge, merge request pipelines that run the source branch's CI file, merges that do not wait for a pipeline that succeeded; GitLab only, unknown elsewhere | | `markers` | the write identity | a pull request carries a marker with this hub's `id` but another fingerprint, or touchmark's marker from an unknown author | | `access` | each target | the writer cannot write there; on GitHub, the App's installation has more permissions than touchmark asks for | | `rules` | each target | a branch rule would block the sync branch or require what touchmark cannot do | diff --git a/docs/guide/setup.md b/docs/guide/setup.md index 2a0a772..5d7e7c1 100644 --- a/docs/guide/setup.md +++ b/docs/guide/setup.md @@ -21,7 +21,12 @@ with the checks of `doctor --hub-token`. Everything goes through the API. The token needs the `api` scope and must belong to a Maintainer of the hub and an Owner of the group whose projects are the targets (or to an -administrator). The hub must live outside that group. The final check also reads the +administrator). The hub must live outside that group, unless `hub.yml` says +`security.writer_on_hub: guard`: then setup accepts a hub inside it, checks that the writer +is below Maintainer on the hub (`writer-guard`), and also turns on *Pipelines must succeed*, +turns off *Skipped pipelines are considered successful* (`merge-checks`), and sets the *CI/CD +configuration file* to `.gitlab-ci.yml@:` (`ci-config`), so that +merge request pipelines run the default branch's CI file. The final check also reads the variables of the groups above the hub, which GitLab shows to their Owners only: for a Maintainer that check stays `unknown`. @@ -32,7 +37,7 @@ setup: create them only when an administrator allows it), group access tokens of the group otherwise. `--accounts` chooses, `--reader-name` and `--writer-name` name them. The reader is a Reporter of the group, the writer a Developer, and the writer must not - reach the hub; + reach the hub (under `writer_on_hub: guard`, not as a Maintainer); - **mints their tokens straight into the hub's variables**: `TOUCHMARK_READ_TOKEN` masked and not protected, since merge request pipelines read it, and `TOUCHMARK_WRITE_TOKEN` protected, masked and hidden, with the environment scope `touchmark-distribute`. A diff --git a/docs/project/e2e.md b/docs/project/e2e.md index 4ee708b..7020078 100644 --- a/docs/project/e2e.md +++ b/docs/project/e2e.md @@ -189,6 +189,7 @@ Secrets never appear on a command line: curl reads its `PRIVATE-TOKEN` header fr That group access token is revoked before `migrate` runs: the bot of a revoked token must still be found and its merge request adopted. - **`TestDoctor`**: `doctor` through `cli.Main` with the writer's token over a fresh subgroup: the token's expiry and scopes (`/personal_access_tokens/self`), a bot or service account (2fa ok); a Developer's access; protected branches over the sync branch, for Maintainers (rules fail) and for Developers without force pushes (warn); another hub's marker with the same id and a person's copy of this hub's (markers warn); a hub the writer does not see (`hub-hidden` ok), then one in its subgroup (fail). `--hub-token` with root's token reads the hub's variables and protected tags: a protected write key scoped to `touchmark-distribute` is ok, an unprotected one fails, a protected tag Developers may create fails. - **`TestSetup`**: `touchmark setup gitlab` through `cli.Main` as a maintainer runs it, with the person's token: an Owner of the seeded group, whose fresh subgroup holds the targets (one project opted in), and an Owner of a separate top-level group where the hub lives (GitLab shows a group's variables, which the final check reads, to its Owners only; a Maintainer gets that check as `unknown` and exit code 3). The hub starts as GitLab makes projects (the default branch protected for Maintainers) and keeps a stale write key, a personal access token of the person that root mints, in an unprotected variable of every environment. The first run must end with no failed, manual or unknown step, and root then reads: the reader and the writer (service accounts where the instance lets the person create them, else group access tokens; the `FINDING` line says which) with Reporter and Developer in the group and no access to the hub; `TOUCHMARK_READ_TOKEN` masked and not protected with scope `*`; `TOUCHMARK_WRITE_TOKEN` protected, masked and hidden with scope `touchmark-distribute`, and the stale copy gone, its token revoked (`GET /personal_access_tokens/self` with it answers 401); the environment; `main` with push No one, merge Maintainers and no force push; no pipeline variables; `protect_merge_request_pipelines` false; the two schedules. A second run and a dry run have only `ok` steps and rotate no token (the active tokens are the same before and after). `doctor --hub-token` with the person's token grades the hub with no `fail` or `warn`. With the runner, a merge request (merged by the person: push to `main` is No one now) names the writer in `hub.yml` and adds a pipeline whose jobs on `main` must succeed: one without an environment runs `probe` (the write key is not visible there) and `plan` (the reader's token reads the group), one with `environment: {name: touchmark-distribute, action: prepare}` runs `doctor` with the writer's token. No token appears in any output or job log. +- **`TestWriterOnHub`**: `security.writer_on_hub: guard` on a hub in a fresh subgroup of the seeded group, which the writer reaches as a Developer: `main` protected (push No one, merge Maintainers), *Pipelines must succeed* without skipped pipelines, and the CI file read from `main` (`ci_config_path` `.gitlab-ci.yml@:main`). `doctor` with the writer's token grades `hub-guard` ok (and no `hub-hidden`). The person's merge request runs the default branch's `plan` on the runner, which passes. The writer then pushes to that merge request's branch a CI file of its own without `plan`: the pipeline still runs `main`'s, and `plan` exits 2 naming the writer's push. A merge request the writer opens fails the same way. With the CI file read from the source branch again, `hub-guard` fails (exit 1). - **`TestProtectedBranch`**: wildcard protected branches, which the writer, a Developer, reads before it pushes. One target protects `touchmark/*` for Maintainers alone: a dry run and the first run block it `blocked:rules:protected-branch` before any write, with a warning naming the rule. Another lets Developers push but not force-push: the first run opens a merge request, and after a pack change the rebuilt branch needs a force push, which is refused, so the target is `blocked:rules:protected-branch` and its merge request stays as it was. A third has two rules on the sync branch, `touchmark/*` for Maintainers alone and the branch's own name for Developers with force pushes: GitLab applies the most permissive rule that matches a branch, so it is delivered and updated as if unprotected. - **`TestFacts`**: GitLab behavior touchmark relies on that the documentation leaves open, through the API and git alone: 409 on a duplicate merge request; draft title prefixes and a `Draft:` commit; labels created by name and `add_labels`; quick actions in a person's description on create and update, against touchmark-shaped text; `closed_by` for a person and for a bot, and `merged_by`; a deleted source branch closes the merge request, who GitLab names as the closer (`closed_by` and `resource_state_events`), and what a reopen without the branch does; a force push keeps it open; how soon after a git push the merge request API takes the new branch (`push-then-create`, see below); `repository_object_format` and sha256 projects; git over HTTP with Basic `oauth2:` and other user names; git with a group access token that has the scope `api` alone; descriptions of 200 000 bytes and around 1 MiB; the reader's `members/all` of the writer; rate-limit headers of a default instance; the bot of a revoked group access token, watched for 90 s (the user, and the author of its merge request); which users API view carries the `bot` field; what a fork of a project the reader cannot read shows; `GET /personal_access_tokens/self`; `push_rule` on CE; the pagination headers. diff --git a/docs/project/threat-model.md b/docs/project/threat-model.md index 4527188..5ad1f12 100644 --- a/docs/project/threat-model.md +++ b/docs/project/threat-model.md @@ -23,7 +23,7 @@ How to read the tests column: |---|---|---|---| | T1. Someone who can push a branch to the hub runs its workflows | The write key reaches the default branch only: on GitHub the environment `touchmark-distribute` with a deployment policy for the default branch alone, which `distribute` reads through the hub channel and refuses otherwise, failing closed (`distribute.CheckEnvironment`); on GitLab a protected variable scoped to that environment, and `distribute` needs `CI_COMMIT_REF_PROTECTED=true` and `CI_ENVIRONMENT_NAME=touchmark-distribute`; Gitea and Forgejo cannot isolate it, so `platform` is refused there. The isolation probe ([below](#isolation-of-the-write-key)). Guards (`distribute.DistributeGuard`): the default branch only, never a `pull_request*` or `merge_request_event` run, a dry run included; operation flags in CI exit 2; `plan` in CI exits 2 when it sees any write key or signing key (`cli.writeGuard`); a hub HEAD that is not the tip of the default branch writes nothing. An untrusted run (a hub pull request, another branch, a tag) takes `providers` and `ca_file` from the default branch, plans a new or moved provider without credentials, and unites the sensitive paths of both. `plan` holds no write credential. One-off operations come only from a reviewed `.touchmark/operations.yml`. | `TestDistributeGuard`, `TestDistributeRefusals`, `TestDistributeLocalRefusals`, `TestPlanRefusals`, `TestCheckEnvironment`, `TestDistributeEnvironment` *(git 2.45)*, `TestProbe`, `TestProbeCommand`, `TestCheckProbe`, `TestCheckProbeProviders`, `TestUntrustedRun`, `TestPlanHubPullRequestProviders`, `TestPlanDefaultBranchProviders`, `TestPlanSensitivePathsUnion` *(git 2.45)*, `TestPlanReachesNoWriter`, `TestMigrateReachesNoWriter`, `TestHeadGuard`, `TestPlanSuperseded`, `TestDistributeScheduleChecksHead` *(git 2.45)*, `TestDistributeChannelFails` *(git 2.45)*, `TestDistributeIsolationNone` *(git 2.45)*, `TestCI` *(e2e GitLab: the probe sees an unprotected scoped variable)* | What the reader reads: on GitHub code and pull requests; on GitLab (Reporter) code, merge requests, issues, CI logs and artifacts (not checked one by one), but not Secure Files or Terraform state; on Gitea code, pull requests and issues. A manual `distribute` on the default branch is the same as the schedule. A read credential that turns out to be the writer's account stops `plan` in CI with exit 2 and is a warning locally. The probe on GitHub is a job of the hub's own workflow: `check` verifies that it tests every provider's write key, not that the template's job stays without an environment. | | T2. A harmful pack gets through a weak review | The ⚠ section of every pull request body: the built-in sensitive paths, hub.yml `sensitive_paths` and any file written with mode 100755, matched ignoring case, deletions included (`prbody.Sensitive`). Agent skills and commands are among the built-in paths (`.claude/skills/**`, `.claude/commands/**`, `.agents/skills/**`, `.github/skills/**`): a skill's `allowed-tools` lets Claude Code use those tools without asking, even in a folder nobody trusted, and its frontmatter can register hooks. `plan` marks them SENSITIVE in "This hub pull request changes"; in a hub pull request the patterns are those of the default branch and of the pull request together, so a pull request cannot drop a pattern and change a file under it unmarked. A new `.gitea/workflows` or `.forgejo/workflows` on a Gitea or Forgejo target that has `.github/workflows` is called out in the body and on the target's line of `plan`: it turns those workflows off. No automerge. The hub template puts CODEOWNERS on `packs/`. | `TestSensitive`, `TestRenderSensitivePending`, `TestCheckCodeowners`, `TestPlanPaths` *(git 2.45)*, `TestPlanSensitivePathsUnion` *(git 2.45)*, `TestRowsAndGiteaWorkflows`, `TestPlanWarnsGiteaWorkflows` *(git 2.45)*, `TestNoMergeRequests` | The sensitive list is a hint, not a boundary: a target's CI runs its `Makefile`, `conftest.py` and the rest too. The sync pull request runs in the target's CI with its secrets before review. Targets should put CODEOWNERS on the sensitive paths. The hub's own CODEOWNERS protect the packs only once they name a real team: `check` warns while one still names the template's `@acme/hub-maintainers`, which the platforms take for no owner. | -| T3. The write key leaks | GitHub: the App's installation token is minted per target, for minutes, with only the permissions the target's work needs (Workflows only when a push touches `.github/workflows`), and revoked when the target is done (`DELETE /installation/token`). `doctor` fails or warns on App permissions beyond contents, pull requests, workflows and metadata. The key never reaches argv, a URL or `.git/config` ([below](#tokens-and-git)). | `TestTargetToken`, `TestTargetTokenRenewal`, `TestTargetApp`, `TestMintWider`, `TestMintRefused`, `TestCloseRevokes`, `TestNeedPerms`, `TestExecuteRevokesTargetWriters`, `TestAppPermissions`, `TestCheck`, `TestTokenLifetime` *(GitHub fake)*, `TestCanaries` | Until the key is revoked: pull requests into every target, a push of any branch and a CI run in every target with its unprotected secrets. The App's private key and the GitLab and Gitea tokens live long; only GitHub's per-target tokens are short. `doctor` does not check the scopes of every kind of token yet. On GitHub the bedrock-python hub runs the live path; what only a live GitHub confirms is listed in [e2e.md](e2e.md#what-only-a-live-github-confirms). | +| T3. The write key leaks | GitHub: the App's installation token is minted per target, for minutes, with only the permissions the target's work needs (Workflows only when a push touches `.github/workflows`), and revoked when the target is done (`DELETE /installation/token`). `doctor` fails or warns on App permissions beyond contents, pull requests, workflows and metadata. The key never reaches argv, a URL or `.git/config` ([below](#tokens-and-git)). | `TestTargetToken`, `TestTargetTokenRenewal`, `TestTargetApp`, `TestMintWider`, `TestMintRefused`, `TestCloseRevokes`, `TestNeedPerms`, `TestExecuteRevokesTargetWriters`, `TestAppPermissions`, `TestCheck`, `TestTokenLifetime` *(GitHub fake)*, `TestCanaries` | Until the key is revoked: pull requests into every target, a push of any branch and a CI run in every target with its unprotected secrets. The App's private key and the GitLab and Gitea tokens live long; only GitHub's per-target tokens are short. `doctor` does not check the scopes of every kind of token yet. On GitHub the bedrock-python hub runs the live path; what only a live GitHub confirms is listed in [e2e.md](e2e.md#what-only-a-live-github-confirms). A GitLab writer that reaches the hub (`security.writer_on_hub: guard`) cannot change the packs only while the guard holds ([below](#isolation-of-the-write-key)). | | T4. A touchmark release is compromised | The hub pins the Action by commit (Dependabot) and, on GitLab, Gitea and Forgejo, the image by digest (Renovate), and takes updates through reviewed pull requests. A release is published by `publish.yml` on master only, for a tag `vX.Y.Z` whose commit is on master and carries the version in `.release-please-manifest.json` and `action.yml`. The workflow checks that with master's copy of `scripts/release/check-version.sh`, never the tag's, and every later job checks out that commit by its id and stops on anything else. The archives and their SBOMs are listed in `checksums.txt`, which cosign signs, and attested; the image is built from the release's own Linux archives once that signature checks out (`scripts/release/build-image.sh`), pushed by digest, checked (`scripts/release/check-image.sh`), attested (the attestation in GHCR too) and signed before any image tag names it. Only the jobs that sign and attest can request an OIDC token; the vulnerability scanner runs in a job of its own with read access alone. binfmt, BuildKit and BuildKit's SBOM scanner are pinned by digest, and no Actions cache is used. The Action at `vX.Y.Z` carries the version, not a digest (`scripts/action/run.sh`): it resolves `ghcr.io/bedrock-python/touchmark:X.Y.Z` to a digest once, verifies that digest's build provenance (`gh attestation verify --bundle-from-oci`: signer `publish.yml` of this repository, source ref `refs/heads/master`, GitHub-hosted runner), checks the image's version label, and runs it by that digest; `GH_TOKEN` never reaches the container. The image holds touchmark and git only, without git-lfs, as a non-root user (`Dockerfile`, `scripts/release/check-image.sh`). A new version's first run is the `plan` of its update pull request, which holds no write credential. Dependabot's pull request gets no Actions secrets, so that `plan` runs offline: it reaches no provider, reports the hub's side and warns, and exits 0; the template passes `--strict` only to pull requests that get secrets. Renovate's merge request on GitLab runs `plan` with the reader's token: GHCR gives no release dates, so the template's `renovate.json` asks for approval on the Dependency Dashboard before it opens one. | `TestPlanReachesNoWriter`, `TestMigrateReachesNoWriter`, `TestPlanOffline`, `TestTemplateWorkflow` *(GitHub fake: Dependabot's pull request; the Action's step resolves, verifies and runs the release's digest)*; the release and Action checks are scripts, not Go tests: `scripts/action/run_test.sh` (a version tag that resolves to nothing, an attestation that fails, an older release's image under the tag, version 0.0.0, `GH_TOKEN` kept out of the container), `scripts/release/tools_test.sh` (a tag off master, a manifest or `action.yml` of another version, the tag's commit checked from a checkout of master, an archive that does not match `checksums.txt`), the CI job `release-snapshot` (the Action against a registry on the runner) and the `verify` job of `publish.yml` (the published release, the Action of the tag run anonymously) | A hub that merges updates without reading them. The offline `plan` checks no target: the maintainer runs `plan` with the reader's key before merging an engine update. The Action trusts a version tag in GHCR only as far as the attestation goes: whoever can move tags of the package (this repository's workflows with `packages: write`, the organization's package admins) can point `X.Y.Z` at another image that `publish.yml` built on master; the label check refuses another version's image, not a rebuild of the same version by a re-run of Publish, which builds the same commit. A tag moved to an image the workflow did not build fails the step: the hub's job stops, nothing runs. Between Release Please's tag and the end of Publish (minutes), or until a failed Publish is run again, the Action at the new tag finds no image and fails, Dependabot's pull request in that window included. A vulnerability the scan finds at publish time does not hold the image back, since the tag exists by then: the scan that blocks is CI's, on the release pull request and nightly, and a red scan at publish is fixed by a patch release. At run time the Action needs `gh`, `docker buildx`, GHCR and Sigstore's trust root: an outage of either stops the hub's jobs, closed. On GitLab, Gitea and Forgejo, verifying the image's signature and attestation before pinning its digest is up to the hub. | | T5. A harmful target | touchmark runs git alone, without a shell ([below](#tokens-and-git)), never checks a target out, and runs no hook, filter, LFS or submodule of a target. Every git in a target's clone is isolated: empty home and global config, `GIT_CONFIG_NOSYSTEM`, `GIT_ATTR_NOSYSTEM`, hooks in an empty directory, `core.fsmonitor=false`, `protocol.allow=never` but https (and http to a loopback host), `http.followRedirects=false`, `transfer.fsckObjects=true`, `submodule.recurse=false`, no credential helper, `GIT_NO_LAZY_FETCH=1`. Limits on what a target can feed it ([below](#untrusted-input)). Paths a target's `.gitattributes` would filter or renormalize leave the change. Only the `recreate` and `repropose` checkboxes of a body act; the rest is data, and bodies touchmark keeps are made inert. | `TestOnlyGitRuns`, `TestTargetIsolation`, `TestInitTarget`, `TestInitTargetDirAndHome`, `TestAttrsPinnedSettings`, `TestReadBlobNeverFetches`, `TestFetchRefusesMalformedObjects`, `TestObjectLimits`, `TestDecodeLimits`, `TestParseGzipBomb`, `TestMaxBody`, `TestRunUnsafePaths` *(git 2.45)*, `TestRunUnsafeFixpoint` *(git 2.45)*, `TestTicked`, `TestInert`, `TestRunKeptBodyIsInert` *(git 2.45)*, `TestCI` *(e2e GitLab: CI_JOB_TOKEN)* | A target may forge its own memory (close, edit or reopen touchmark's pull requests): it is the target's right. GitLab: the CI of a sync branch runs as the writer, and its `CI_JOB_TOKEN` acts with the writer's access to other targets. The job token allowlist, on for new projects and enforced instance-wide from 18.x, keeps target X's token away from target Y through the API and git; if Y lists X, the token reads Y's code (17.11 git only; 18.11 and 19.4 also files and merge requests through the API), but push and API writes are refused (checked live on GitLab 17.11, 18.11 and 19.4). Mitigation: never list sync targets in an allowlist, and a separate writer per group of targets with another level of trust. | | T6. Another hub with the same `id` | The hub's fingerprint (host and immutable repository id) is in every commit's trailers and every marker; a sync branch whose touchmark commits carry another fingerprint is Foreign and `blocked:branch-taken`; a pull request with another hub's marker is not ours. `doctor` warns about markers with our `id` and another fingerprint. | `TestFindDataOfAnotherHub`, `TestFindWithoutFingerprints`, `TestOwn`, `TestOwnWithoutFingerprints`, `TestClassifyBranch`, `TestParseTrailers`, `TestCanonicalFingerprint`, `TestDoctor` *(e2e Gitea: markers warn)* | Two hubs with one `id` share the branch name `touchmark/`: the second one sets `branch` in hub.yml. | @@ -44,6 +44,7 @@ How to read the tests column: | `check` requires every secret a workflow hands touchmark as a write key, `with:` inputs of the Action included, to be tested by every probe expression and by the job whose output `TOUCHMARK_KEY_EXPOSED` reads (a test in another job does not count; a constant is refused), and the write key of every provider to be handed out where it can see it | `config.CheckProbe` | `TestCheckProbe`, `TestCheckProbeProviders`, `TestWriteKeySecrets` | `check` reads `.github/workflows` only, and does not follow a `TOUCHMARK_KEY_EXPOSED` that a reusable workflow takes from its inputs. | | GitLab: `touchmark probe` in merge request pipelines exits 2 when it sees a write key | `distribute.Probe`, `cli` probe command | `TestProbe`, `TestProbeCommand`, `TestCI` *(e2e GitLab)* | `distribute` cannot see a variable's flags; the probe in every merge request pipeline is what catches it. | | Bitbucket Pipelines and Azure Pipelines: `distribute` and `doctor` need the environment `touchmark-distribute` (the step's deployment, a deployment job) and the default branch read by the hub channel; under `platform` they need `security.reason` stating what keeps other branches out (Premium deployment permissions; the variable group's Branch control check), which the job cannot read | `distribute.guardContextOf`, `distribute.guardIsolation`, `distribute.IsolationChecks`, `cli.hubRepository` | `TestDistributeGuardBitbucket`, `TestIsolationChecksBitbucket`, `TestDistributeGuardAzure`, `TestIsolationChecksAzure`, `TestHubRepositoryBitbucket`, `TestHubRepositoryAzure`, `TestTemplatePipelines`, `TestTemplateAzurePipelines` | The statement is the hub's word: `doctor --hub-token` reads the Azure check with a maintainer's token, and nothing reads Bitbucket's deployment permissions. | +| GitLab, `security.writer_on_hub: guard`: a writer that reaches the hub as a Developer cannot get content onto its default branch. `doctor`'s `hub-guard` checks its role (below Maintainer), the rules that match the default branch (no push, no merge for it), that merge request pipelines read the CI file of the default branch (`ci_config_path` `@:`, so a pushed CI file cannot drop the plan), and that a merge waits for a pipeline that succeeded and not a skipped one; `doctor --hub-token` grades the settings with a maintainer's token; `setup gitlab` sets them up. `plan` in a hub merge request pipeline exits 2 when the writer or a known author opened it or pushed to its source branch since its creation (push events, which name the pusher), and when it cannot tell; the setting, the writer and the known authors come from the default branch's `hub.yml` | `platform/gitlab.GuardHub`, `platform/gitlab.MergeRequestPushes`, `distribute.hubGuardCheck`, `distribute.gitlabGuardSettings`, `cli.writerMergeGuard`, `setup.writerGuard`, `setup.guardSettings` | `TestGuardHub`, `TestMergeRequestPushes`, `TestDoctorHubGuard`, `TestKeyLocationGuardSettings`, `TestWriterMergeGuard`, `TestGitLabWriterOnHub`, `TestPinnedCIConfig`, `TestWriterOnHub` *(e2e GitLab)* | A maintainer who merges a merge request without reading it; settings changed between two `doctor` runs; push rules (EE) that show only at push time; protected tags, which GitLab shows to Maintainers only, are graded by `doctor --hub-token` alone. Branch protection and merge checks are the hub's to keep: the guard verifies, it does not enforce. | | Gitea and Forgejo cannot isolate the key: `platform` refused, `doctor` fails | `distribute.guardIsolation`, `distribute.IsolationChecks` | `TestDistributeGuard`, `TestIsolationChecks` | A hub on Gitea or Forgejo runs with `none` and a reason, or its CI runs on GitHub or GitLab. | | `touchmark setup` offers only an isolated write key: it refuses `security.write_isolation: none`, Gitea and Forgejo, and CI; it refuses a GitLab hub inside the group of targets and fails when the writer reaches the hub | `cli.runSetup`, `setup.GitLab`, `setup.GitHub` | `TestSetupRefusals`, `TestGitLabPreconditions`, `TestGitLabWriterReachesHub`, `TestGitHubPreconditions`, `TestGitHubFreePrivate` | — | | GitLab: setup mints the tokens straight into the hub's variables, registered with the redaction before use and never printed; the write key protected, masked, hidden and scoped to `touchmark-distribute`. A write key found unprotected, unmasked or in another scope: setup mints a new token (rotating its own revokes the old one), revokes the token each exposed variable held with that token itself (`DELETE /personal_access_tokens/self`), whoever made it, and deletes the copies; a refused revocation fails the step `writer-exposed-token` | `setup.gitlabSetup` (`mint`, `writerVariable`, `revokeExposed`) | `TestGitLabGroupTokens`, `TestGitLabServiceAccounts`, `TestGitLabRepairsWriteKey`, `TestGitLabStoreFails`, `TestSetup` *(e2e GitLab: a token made by hand in a variable of every environment is revoked)* | GitLab hides a variable only when it creates it: a protected, masked write key made by hand before setup stays readable by Maintainers in the UI (setup warns). The value of a hidden exposed copy cannot be read: setup cannot revoke its token and asks the person to (a manual step). | diff --git a/docs/reference/hub.md b/docs/reference/hub.md index cff2f2f..c96b72b 100644 --- a/docs/reference/hub.md +++ b/docs/reference/hub.md @@ -105,6 +105,7 @@ In a hub pull request, `providers` and `ca_file` are read from the default branc | `security.write_isolation` | one of `platform`, `external`, `none` | Where the write key lives. platform: in the hub's CI platform, checked by a probe (on Bitbucket Pipelines and Azure Pipelines only with a reason); external: issued by an external vault over OIDC; none: risk accepted, needs a reason. Default: platform. | | `security.reason` | string | Why the write key cannot be isolated. Required with write_isolation: none, and with platform on Bitbucket Pipelines, where it states the deployment permissions (Premium) that keep the write key on the default branch, and on Azure Pipelines, where it states the Branch control check of the variable group that holds it: touchmark cannot read either. | | `security.private_targets_in_public_hub` | one of `skip`, `deliver` | What a public hub does with non-public targets: skip them, or deliver and let their names reach public CI logs. Default: skip. | +| `security.writer_on_hub` | one of `refuse`, `guard` | What a writer that reaches the hub means. refuse: doctor fails it, and setup gitlab refuses a hub inside the group of targets. guard (GitLab): the writer may be a Developer of the hub; doctor verifies that it cannot get content onto the default branch, and plan refuses a hub merge request the writer opened or pushed to. Default: refuse. | | `providers` | list of objects | Platforms this hub delivers to. Targets refer to them by id. | | `providers[].id` | string, required | Name targets.yml uses for this provider, and the <ID> of its TOUCHMARK_<ID>_\* variables. | | `providers[].type` | one of `github`, `gitlab`, `gitea`, `forgejo`, `bitbucket`, `azure-devops`, required | github: github.com, GHE.com or GitHub Enterprise Server; gitlab; gitea; forgejo; bitbucket: Bitbucket Cloud; azure-devops: one organization of Azure DevOps Services (bitbucket and azure-devops: plan, distribute and doctor; setup does not set up a hub on them yet). | diff --git a/internal/cli/plan.go b/internal/cli/plan.go index 340df6a..9c49078 100644 --- a/internal/cli/plan.go +++ b/internal/cli/plan.go @@ -194,6 +194,11 @@ func runPlan(ctx context.Context, e *env, o *options, p *planOptions) (err error } providers[i] = distribute.Provider{Config: pp.ResolvedProvider, Reader: reader, Anonymous: pp.anonymous} } + guardWarnings, err := h.writerMergeGuard(ctx, hctx, e.getenv, pps, providers) + warnings = append(warnings, guardWarnings...) + if err != nil { + return err + } channel := h.hubChannel(ctx, hctx, pps, token, reg) hctx, visWarnings := channelVisibility(ctx, hctx, channel) warnings = append(warnings, visWarnings...) diff --git a/internal/cli/plan_guard.go b/internal/cli/plan_guard.go new file mode 100644 index 0000000..67399bd --- /dev/null +++ b/internal/cli/plan_guard.go @@ -0,0 +1,107 @@ +package cli + +import ( + "context" + "fmt" + "slices" + "strings" + + "github.com/bedrock-python/touchmark/internal/config" + "github.com/bedrock-python/touchmark/internal/distribute" + "github.com/bedrock-python/touchmark/internal/hubch" + "github.com/bedrock-python/touchmark/internal/platform" +) + +// writerMergeGuard is plan's guard of a hub merge request under +// security.writer_on_hub guard (GitLab): the writer reaches the hub as a +// Developer, so it may open a merge request or push to the source branch +// of a maintainer's, and a leaked write key could slip a change into what +// a maintainer then merges. In a merge request pipeline of a hub whose +// default branch's hub.yml says guard, plan exits 2 when the merge request's +// author, or anyone who pushed to its source branch since the branch was +// created, is the writer of the provider on the hub's host or one of its +// known_authors; when that cannot be told (no such provider, a reader +// without the answer, events that do not reach the branch's creation), it +// exits 2 too: the guard fails closed. With "Pipelines must succeed" and +// no merge on a skipped pipeline, and the CI configuration read from the +// default branch (which doctor's hub-guard verifies), the merge waits. +// +// The setting, the writer and known_authors come from the default branch's +// hub.yml: the merge request's own could turn the guard off. When the +// default branch's cannot be read, the run's own decides, with a warning. +func (h *hub) writerMergeGuard(ctx context.Context, hctx hubch.Context, getenv func(string) string, pps []planProvider, + providers []distribute.Provider) ([]string, error) { + pr := hubPR(hctx, getenv) + if hctx.CI != hubch.GitLabCI || hctx.Event != "merge_request_event" || pr <= 0 { + return nil, nil + } + cfg, warnings := h.cfg, []string(nil) + tip, err := h.defaultBranchTip(ctx, hctx) + if err == nil { + var base *config.Hub + if base, err = h.hubAt(ctx, tip); err == nil { + cfg = base + } + } + if err != nil { + warnings = append(warnings, fmt.Sprintf("security.writer_on_hub is read from this merge request's %s: %v", config.HubFile, err)) + } + if cfg == nil || cfg.Security.WriterOnHub != "guard" { + return warnings, nil + } + refuse := func(format string, args ...any) error { + return configErrorf("security.writer_on_hub guard: "+format, args...) + } + i := slices.IndexFunc(pps, func(pp planProvider) bool { return strings.EqualFold(pp.Host, hctx.Host) && pp.Type == "gitlab" }) + if i < 0 { + return warnings, refuse("no GitLab provider of %s is on the hub's host %s, so plan cannot tell who pushed to !%d", config.HubFile, hctx.Host, pr) + } + auditor, ok := providers[i].Reader.(platform.MergeRequestAuditor) + if !ok || pps[i].anonymous { + return warnings, refuse("provider %s has no reader with a token here, so plan cannot tell who pushed to !%d", pps[i].ID, pr) + } + writer := pps[i].Writer + if cfg.Writer != "" && len(cfg.Providers) == 0 { + writer = cfg.Writer + } + if writer == "" { + return warnings, refuse("provider %s names no writer, so plan cannot tell whether it pushed to !%d", pps[i].ID, pr) + } + logins := append([]string{writer}, pps[i].KnownAuthors...) + var accounts []platform.Account + for _, login := range logins { + a, err := providers[i].Reader.Lookup(ctx, login) + if err != nil { + return warnings, refuse("look up %s on %s: %v", login, pps[i].ID, err) + } + accounts = append(accounts, a) + } + hub := platform.Repo{Host: hctx.Host, ID: hctx.RepoID, Path: hctx.RepoPath} + got, err := auditor.MergeRequestPushes(ctx, hub, pr) + if err != nil { + return warnings, refuse("read !%d and the pushes to its source branch: %v", pr, err) + } + match := func(a platform.Account) (string, bool) { + for _, w := range accounts { + if a.ID != "" && a.ID == w.ID { + return w.Login, true + } + } + return "", false + } + if login, ok := match(got.Author); ok { + return warnings, refuse("!%d was opened by %s, the writer or one of its known authors: a leaked write key must not reach the hub's default "+ + "branch; a maintainer opens it again from a branch of their own", pr, login) + } + for _, p := range got.Pushers { + if login, ok := match(p); ok { + return warnings, refuse("%s, the writer or one of its known authors, pushed to %s, the source branch of !%d: a leaked write key must not "+ + "reach the hub's default branch; a maintainer recreates the branch from commits they checked", login, got.Branch, pr) + } + } + if !got.Complete { + return warnings, refuse("plan cannot tell everyone who pushed to %s, the source branch of !%d (%s): the guard fails closed; "+ + "push the change to a new branch and open the merge request again", got.Branch, pr, got.Why) + } + return warnings, nil +} diff --git a/internal/cli/plan_guard_test.go b/internal/cli/plan_guard_test.go new file mode 100644 index 0000000..7020b5e --- /dev/null +++ b/internal/cli/plan_guard_test.go @@ -0,0 +1,141 @@ +package cli + +import ( + "context" + "errors" + "os" + "strings" + "testing" + + "github.com/bedrock-python/touchmark/internal/config" + "github.com/bedrock-python/touchmark/internal/distribute" + "github.com/bedrock-python/touchmark/internal/hubch" + "github.com/bedrock-python/touchmark/internal/platform" +) + +// auditReader is a GitLab reader that answers Lookup by login and +// MergeRequestPushes with pushes. +type auditReader struct { + platform.Reader + accounts map[string]platform.Account + pushes platform.MergeRequestPushes + asked int +} + +func (r *auditReader) Lookup(_ context.Context, login string) (platform.Account, error) { + a, ok := r.accounts[login] + if !ok { + return platform.Account{}, platform.ErrNotFound + } + return a, nil +} + +func (r *auditReader) MergeRequestPushes(_ context.Context, hub platform.Repo, number int64) (platform.MergeRequestPushes, error) { + r.asked++ + if hub.ID != "21" || number != 5 { + return platform.MergeRequestPushes{}, errors.New("another merge request") + } + return r.pushes, nil +} + +// guardHubRepo is a hub whose default branch master says writer_on_hub +// mainMode, checked out on a branch that says branchMode. +func guardHubRepo(t *testing.T, mainMode, branchMode string) *hub { + t.Helper() + hubYML := func(mode string) string { + return "version: 1\nid: acme-eng\nproviders:\n - id: corp\n type: gitlab\n url: https://gitlab.example.com\n" + + " writer: writer-bot\n known_authors: [old-bot]\nsecurity:\n writer_on_hub: " + mode + "\n" + } + r := newHub(t) + r.write(config.HubFile, hubYML(mainMode)) + r.commit("hub") + r.git("checkout", "-q", "-b", "feature") + r.write(config.HubFile, hubYML(branchMode)) + r.commit("feature") + e := &env{getenv: func(string) string { return "" }, stdout: os.Stdout, stderr: os.Stderr, getwd: os.Getwd} + h, err := openHub(t.Context(), e, r.dir, false) + if err != nil { + t.Fatal(err) + } + if errs := h.readConfigs(t.Context(), e); len(errs) > 0 { + t.Fatal(errs) + } + return h +} + +// TestWriterMergeGuard: plan in a GitLab merge request pipeline of a hub +// whose default branch says writer_on_hub guard refuses (exit 2) a merge +// request the writer or a known author opened or pushed to, and one whose +// pushes it cannot tell; the merge request's own hub.yml cannot turn the +// guard off. +func TestWriterMergeGuard(t *testing.T) { + mr := hubch.Context{CI: hubch.GitLabCI, Event: "merge_request_event", Host: "gitlab.example.com", RepoID: "21", + RepoPath: "acme/engineering-assets", DefaultBranch: "master"} + getenv := func(k string) string { + if k == "CI_MERGE_REQUEST_IID" { + return "5" + } + return "" + } + accounts := map[string]platform.Account{"writer-bot": {ID: "7", Login: "writer-bot"}, "old-bot": {ID: "8", Login: "old-bot"}} + person := platform.Account{ID: "3", Login: "jdoe"} + pps := []planProvider{{ResolvedProvider: config.ResolvedProvider{Provider: config.Provider{ID: "corp", Type: "gitlab", + Writer: "writer-bot", KnownAuthors: []string{"old-bot"}}, Host: "gitlab.example.com"}}} + run := func(h *hub, c hubch.Context, pushes platform.MergeRequestPushes) (*auditReader, error) { + r := &auditReader{accounts: accounts, pushes: pushes} + _, err := h.writerMergeGuard(t.Context(), c, getenv, pps, []distribute.Provider{{Reader: r}}) + return r, err + } + clean := platform.MergeRequestPushes{Author: person, Branch: "feature", Pushers: []platform.Account{person}, Complete: true} + for _, tc := range []struct { + name string + main string + branch string + pushes platform.MergeRequestPushes + want string + }{ + {"clean", "guard", "guard", clean, ""}, + {"opened by the writer", "guard", "guard", platform.MergeRequestPushes{Author: platform.Account{ID: "7"}, Branch: "feature", Complete: true}, + "was opened by writer-bot"}, + {"a known author pushed", "guard", "guard", platform.MergeRequestPushes{Author: person, Branch: "feature", + Pushers: []platform.Account{person, {ID: "8"}}, Complete: true}, "old-bot, the writer or one of its known authors, pushed to feature"}, + {"pushes unknown", "guard", "guard", platform.MergeRequestPushes{Author: person, Branch: "feature", Why: "no creation"}, "fails closed"}, + // The merge request's own hub.yml cannot turn the guard off. + {"turned off in the merge request", "guard", "refuse", platform.MergeRequestPushes{Author: platform.Account{ID: "7"}, Complete: true}, + "was opened by"}, + {"off on the default branch", "refuse", "guard", platform.MergeRequestPushes{Author: platform.Account{ID: "7"}, Complete: true}, ""}, + } { + t.Run(tc.name, func(t *testing.T) { + h := guardHubRepo(t, tc.main, tc.branch) + _, err := run(h, mr, tc.pushes) + switch { + case tc.want == "" && err != nil: + t.Errorf("error %v", err) + case tc.want != "" && (err == nil || !strings.Contains(err.Error(), tc.want)): + t.Errorf("error %v, want %q", err, tc.want) + case tc.want != "": + var ce *cliError + if !errors.As(err, &ce) || ce.code != exitUsage { + t.Errorf("error %v: want exit %d", err, exitUsage) + } + } + }) + } + h := guardHubRepo(t, "guard", "guard") + // Not a merge request pipeline: nothing is asked. + for _, c := range []hubch.Context{{CI: hubch.GitLabCI, Event: "push"}, {CI: hubch.GitHubActions, Event: "pull_request"}} { + if r, err := run(h, c, clean); err != nil || r.asked != 0 { + t.Errorf("%+v: %v, asked %d", c, err, r.asked) + } + } + // A reader without the answer, or without a token, fails closed. + if _, err := h.writerMergeGuard(t.Context(), mr, getenv, pps, []distribute.Provider{{Reader: offlineReader{}}}); err == nil || + !strings.Contains(err.Error(), "no reader with a token") { + t.Errorf("no auditor: %v", err) + } + anon := []planProvider{pps[0]} + anon[0].anonymous = true + if _, err := h.writerMergeGuard(t.Context(), mr, getenv, anon, []distribute.Provider{{Reader: &auditReader{accounts: accounts, pushes: clean}}}); err == nil { + t.Error("an anonymous provider passed") + } +} diff --git a/internal/cli/setup.go b/internal/cli/setup.go index 36e6dc4..bc5491e 100644 --- a/internal/cli/setup.go +++ b/internal/cli/setup.go @@ -130,10 +130,13 @@ func runSetup(ctx context.Context, e *env, o *options, s *setupOptions) (err err return configErrorf("%w\nfix %s first (touchmark check --worktree tells more)", joinedError(err), config.HubFile) } } - isolation := "platform" + isolation, writerOnHub := "platform", "" if cfg != nil && cfg.Security.WriteIsolation != "" { isolation = cfg.Security.WriteIsolation } + if cfg != nil { + writerOnHub = cfg.Security.WriterOnHub + } if isolation == "none" { return configErrorf("setup offers only an isolated write key, and %s sets security.write_isolation: none: "+ "set it to platform (or external) to use setup, or keep the keys by hand: %s", config.HubFile, docsurl.GettingStarted(platform)) @@ -174,7 +177,7 @@ func runSetup(ctx context.Context, e *env, o *options, s *setupOptions) (err err case "gitlab": rep, err = setup.GitLab(ctx, setup.GitLabInput{ APIURL: trusted.APIURL, Host: rp.Host, Project: repoPath, Group: strings.Trim(s.group, "/"), Token: token, Client: client, - Isolation: isolation, Accounts: s.accounts, ReaderName: s.readerName, WriterName: s.writerName, + Isolation: isolation, WriterOnHub: writerOnHub, Accounts: s.accounts, ReaderName: s.readerName, WriterName: s.writerName, ReadVar: prefix + "READ_TOKEN", WriteVar: prefix + "WRITE_TOKEN", Writer: rp.Writer, TokenDays: s.tokenDays, Schedules: s.schedules, DryRun: o.dryRun, Engine: version(), Redact: reg, BasicUsers: basicUsers, diff --git a/internal/config/config.go b/internal/config/config.go index 5d306d8..6d5bc49 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -93,6 +93,12 @@ type Security struct { WriteIsolation string `yaml:"write_isolation"` // platform | external | none Reason string `yaml:"reason"` PrivateTargetsInPublicHub string `yaml:"private_targets_in_public_hub"` // skip | deliver + // WriterOnHub is what touchmark makes of a writer that reaches the hub: + // refuse (the default: doctor fails hub-hidden, setup refuses a hub + // inside the group of targets) or guard (on GitLab, doctor verifies + // that the writer cannot get content onto the hub's default branch, and + // plan refuses a hub merge request the writer opened or pushed to). + WriterOnHub string `yaml:"writer_on_hub"` // refuse | guard } type Provider struct { @@ -234,7 +240,8 @@ func (o *OptIn) Disabled() bool { return o != nil && o.Enabled != nil && !*o.Ena // takes its default: branch touchmark/, the commit message and PR title // "chore: sync engineering assets", the label engineering-assets, link_hub // auto, max_new_prs_per_run 100, max_close_fraction 0.1, auto_close_cooldown -// 30d, write_isolation platform, private_targets_in_public_hub skip, and per +// 30d, write_isolation platform, private_targets_in_public_hub skip, +// writer_on_hub refuse, and per // provider sign auto and the public URL of github and gitlab; the hosts of // previous_fingerprints are canonical (CanonicalFingerprint). The // single-provider shorthand with platform (platform, base_url, writer, sign) diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 6af2372..bec6c4c 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -43,7 +43,7 @@ func TestParseHubDefaults(t *testing.T) { }, Limits: Limits{MaxNewPRsPerRun: 100, MaxCloseFraction: 0.1}, Memory: Memory{AutoCloseCooldown: "30d"}, - Security: Security{WriteIsolation: "platform", PrivateTargetsInPublicHub: "skip"}, + Security: Security{WriteIsolation: "platform", PrivateTargetsInPublicHub: "skip", WriterOnHub: "refuse"}, } if !reflect.DeepEqual(h, want) { t.Errorf("got %+v\nwant %+v", h, want) diff --git a/internal/config/hub.go b/internal/config/hub.go index 65a4b9b..bd1da1d 100644 --- a/internal/config/hub.go +++ b/internal/config/hub.go @@ -21,18 +21,20 @@ const ( defaultAutoCloseCooldown = "30d" defaultWriteIsolation = "platform" defaultPrivateTargets = "skip" + defaultWriterOnHub = "refuse" defaultSign = "auto" ) // Enumerations of hub.yml. var ( - providerTypes = []string{"github", "gitlab", "gitea", "forgejo", "bitbucket", "azure-devops"} - linkHubModes = []string{"auto", "always", "never"} - isolationModes = []string{"platform", "external", "none"} - privateTargets = []string{"skip", "deliver"} - signModes = []string{"auto", "always"} - defaultURLs = map[string]string{"github": "https://github.com", "gitlab": "https://gitlab.com", "bitbucket": bitbucketURL} - shorthandFields = []string{"writer", "sign", "platform", "base_url"} + providerTypes = []string{"github", "gitlab", "gitea", "forgejo", "bitbucket", "azure-devops"} + linkHubModes = []string{"auto", "always", "never"} + isolationModes = []string{"platform", "external", "none"} + privateTargets = []string{"skip", "deliver"} + writerOnHubModes = []string{"refuse", "guard"} + signModes = []string{"auto", "always"} + defaultURLs = map[string]string{"github": "https://github.com", "gitlab": "https://gitlab.com", "bitbucket": bitbucketURL} + shorthandFields = []string{"writer", "sign", "platform", "base_url"} ) func parseHub(data []byte) (*Hub, []Warning, error) { @@ -167,6 +169,7 @@ func checkLimits(l *Limits, doc document, p *problems) { func checkSecurity(s *Security, doc document, p *problems) { checkEnum(p, doc, "security.write_isolation", s.WriteIsolation, isolationModes) checkEnum(p, doc, "security.private_targets_in_public_hub", s.PrivateTargetsInPublicHub, privateTargets) + checkEnum(p, doc, "security.writer_on_hub", s.WriterOnHub, writerOnHubModes) if s.WriteIsolation == "none" && isBlank(s.Reason) { p.errorf("security.reason", "required with write_isolation: none: say why the write key cannot be isolated") } @@ -354,6 +357,7 @@ func applyHubDefaults(h *Hub, doc document) { setDefault(&h.Memory.AutoCloseCooldown, defaultAutoCloseCooldown) setDefault(&h.Security.WriteIsolation, defaultWriteIsolation) setDefault(&h.Security.PrivateTargetsInPublicHub, defaultPrivateTargets) + setDefault(&h.Security.WriterOnHub, defaultWriterOnHub) for i, fp := range h.PreviousFingerprints { h.PreviousFingerprints[i] = CanonicalFingerprint(fp) } diff --git a/internal/config/providers.go b/internal/config/providers.go index 04e869a..64278ae 100644 --- a/internal/config/providers.go +++ b/internal/config/providers.go @@ -599,3 +599,17 @@ func (o *OptIn) Hash() string { } return "sha256:" + hex.EncodeToString(h.Sum(nil)) } + +// PinnedCIConfig reports whether a GitLab CI configuration path reads the +// hub's file from the hub's default branch: "@:", +// or "@", which GitLab reads from the project's default +// branch; the project path compared without case. +func PinnedCIConfig(p, hubPath, branch string) bool { + file, rest, ok := strings.Cut(strings.TrimSpace(p), "@") + if !ok || file == "" || strings.Contains(file, ":") { + return false + } + // Without a ref GitLab reads the project's default branch. + project, ref, hasRef := strings.Cut(rest, ":") + return branch != "" && (!hasRef || ref == branch) && strings.EqualFold(project, hubPath) +} diff --git a/internal/config/providers_test.go b/internal/config/providers_test.go index 09b3483..20b80f3 100644 --- a/internal/config/providers_test.go +++ b/internal/config/providers_test.go @@ -772,3 +772,25 @@ func TestAzureOrigin(t *testing.T) { t.Error("dev.azure.com: an origin there names its organization") } } + +func TestPinnedCIConfig(t *testing.T) { + for p, want := range map[string]bool{ + ".gitlab-ci.yml@acme/hub:main": true, + "ci/hub.yml@Acme/Hub:main": true, + " .gitlab-ci.yml@acme/hub:main ": true, + ".gitlab-ci.yml@acme/hub": true, // the project's default branch + ".gitlab-ci.yml@acme/hub:dev": false, + ".gitlab-ci.yml@acme/other:main": false, + ".gitlab-ci.yml": false, + "": false, + "@acme/hub:main": false, + "https://x.example/ci.yml@acme/hub:main": false, + } { + if got := PinnedCIConfig(p, "acme/hub", "main"); got != want { + t.Errorf("PinnedCIConfig(%q) = %v, want %v", p, got, want) + } + } + if PinnedCIConfig(".gitlab-ci.yml@acme/hub:", "acme/hub", "") { + t.Error("no default branch") + } +} diff --git a/internal/config/testdata/hub/invalid/writer-on-hub-invalid.yml b/internal/config/testdata/hub/invalid/writer-on-hub-invalid.yml new file mode 100644 index 0000000..39910a8 --- /dev/null +++ b/internal/config/testdata/hub/invalid/writer-on-hub-invalid.yml @@ -0,0 +1,5 @@ +# error: security.writer_on_hub: "allow" +version: 1 +id: acme-eng +security: + writer_on_hub: allow diff --git a/internal/config/testdata/hub/valid/writer-on-hub-guard.yml b/internal/config/testdata/hub/valid/writer-on-hub-guard.yml new file mode 100644 index 0000000..95b713a --- /dev/null +++ b/internal/config/testdata/hub/valid/writer-on-hub-guard.yml @@ -0,0 +1,5 @@ +version: 1 +id: acme-eng +writer: touchmark-writer +security: + writer_on_hub: guard diff --git a/internal/distribute/doctor.go b/internal/distribute/doctor.go index 883b925..3c15d83 100644 --- a/internal/distribute/doctor.go +++ b/internal/distribute/doctor.go @@ -57,7 +57,8 @@ type DoctorDeps struct { // - hub-hidden: for the provider on the hub's host, that its writer may // not write to the hub: ok when the hub is not visible to it or it // cannot push there, fail when it can, warn when a private hub is -// visible to it (it is a member). +// visible to it (it is a member). Under security.writer_on_hub guard a +// writer that sees the hub gets hub-guard instead (hubGuardCheck). // // Per target: skipped with the reason of delivery (archived, disabled, // empty, mirror, pending-deletion, prs-disabled, sha256), not-opted-in @@ -253,6 +254,8 @@ func (r *run) hubHiddenCheck(ctx context.Context, p *provider, dd DoctorDeps) re return c case err != nil: return unknownCheck("hub-hidden", "look up the hub as the writer", err) + case r.hub != nil && r.hub.Security.WriterOnHub == "guard": + return r.hubGuardCheck(ctx, p, hub) } checker, ok := p.writer.(platform.Checker) if !ok { @@ -289,6 +292,53 @@ func (r *run) hubHiddenCheck(ctx context.Context, p *provider, dd DoctorDeps) re return c } +// hubGuardCheck is check hub-guard, under security.writer_on_hub guard for +// a writer that sees the hub: the conditions of platform.HubGuard as one +// check, its status the worst of theirs, its detail the conditions that +// are not ok (all of them when every one is). A driver without HubGuard +// gives unknown: only GitLab verifies the guard. +func (r *run) hubGuardCheck(ctx context.Context, p *provider, hub platform.Repo) report.DoctorCheck { + c := report.DoctorCheck{Name: "hub-guard"} + guard, ok := p.writer.(platform.HubGuard) + if !ok { + c.Status, c.Detail = report.StatusUnknown, "the writer sees the hub, and security.writer_on_hub guard is verified on GitLab only, not by the "+ + p.cfg.Type+" driver: keep the writer off the hub" + return c + } + var fs []platform.Finding + err := r.retry(ctx, p, func() error { + var err error + fs, err = guard.GuardHub(ctx, hub) + return err + }) + if err != nil { + return unknownCheck("hub-guard", "check the hub's guard as the writer", err) + } + if len(fs) == 0 { + c.Status, c.Detail = report.StatusUnknown, "the driver reported no condition of the guard" + return c + } + rank := map[report.CheckStatus]int{report.StatusOK: 0, report.StatusUnknown: 1, report.StatusWarn: 2, report.StatusFail: 3} + c.Status = report.StatusOK + var bad, all []string + for _, f := range fs { + s := fromFinding(f).Status + if rank[s] > rank[c.Status] { + c.Status = s + } + all = append(all, f.Detail) + if s != report.StatusOK { + bad = append(bad, string(s)+": "+f.Detail) + } + } + if len(bad) > 0 { + c.Detail = strings.Join(bad, "; ") + } else { + c.Detail = "the writer reaches the hub but cannot get content onto its default branch: " + strings.Join(all, "; ") + } + return c +} + // doctorTargets checks every target of list, in parallel with the pools // of inspectAll (eachByProvider), and returns their report entries in the // order of list. diff --git a/internal/distribute/doctor_guard_test.go b/internal/distribute/doctor_guard_test.go new file mode 100644 index 0000000..199d376 --- /dev/null +++ b/internal/distribute/doctor_guard_test.go @@ -0,0 +1,96 @@ +package distribute + +import ( + "context" + "testing" + + "github.com/bedrock-python/touchmark/internal/hubch" + "github.com/bedrock-python/touchmark/internal/platform" + "github.com/bedrock-python/touchmark/internal/report" +) + +// guardWriter is a writer whose GuardHub answers findings. +type guardWriter struct { + platform.Writer + findings []platform.Finding +} + +func (g guardWriter) GuardHub(context.Context, platform.Repo) ([]platform.Finding, error) { + return g.findings, nil +} + +// TestDoctorHubGuard: under security.writer_on_hub guard, a writer that +// reaches the hub gets hub-guard instead of hub-hidden: the worst of the +// driver's conditions, unknown without HubGuard; a writer that does not +// see the hub keeps hub-hidden. +func TestDoctorHubGuard(t *testing.T) { + ok := platform.Finding{Check: "hub-guard", Status: platform.FindingOK, Detail: "keeps the writer from pushing to main"} + for _, tc := range []struct { + name string + visible bool + findings []platform.Finding + guard bool + check string + status report.CheckStatus + detail string + }{ + {"not visible", false, nil, true, "hub-hidden", report.StatusOK, "does not see the hub"}, + {"no HubGuard", true, nil, false, "hub-guard", report.StatusUnknown, "verified on GitLab only"}, + {"guarded", true, []platform.Finding{ok, ok}, true, "hub-guard", report.StatusOK, "cannot get content onto its default branch"}, + {"a failing condition", true, []platform.Finding{ok, {Check: "hub-guard", Status: platform.FindingFail, Detail: "main is not protected"}, + {Check: "hub-guard", Status: platform.FindingWarn, Detail: "protected branch release"}}, true, "hub-guard", report.StatusFail, + "fail: main is not protected; warn: protected branch release"}, + } { + t.Run(tc.name, func(t *testing.T) { + w := newWorld(t) + w.hubYML += "security:\n writer_on_hub: guard\n" + if tc.visible { + hub := w.repo("acme/engineering-assets", nil) + w.p.GrantWrite(hub.ID, w.writer) + } + w.ok() + dd := w.doctorDeps(w.writer) + if tc.guard { + for i := range dd.Providers { + dd.Providers[i].Writer = guardWriter{Writer: dd.Providers[i].Writer, findings: tc.findings} + } + } + doc := w.doctor(dd) + wantCheck(t, "gh", doc.Providers[0].Checks, tc.check, tc.status, tc.detail) + }) + } +} + +// TestKeyLocationGuardSettings: doctor --hub-token grades, under +// security.writer_on_hub guard, the GitLab hub's CI configuration path +// and merge checks. +func TestKeyLocationGuardSettings(t *testing.T) { + str := func(s string) *string { return &s } + yes, no := true, false + guard := hubWithIsolation("platform", "") + guard.Security.WriterOnHub = "guard" + for _, tc := range []struct { + name string + path *string + after, skipped *bool + status report.CheckStatus + detail string + }{ + {"guarded", str(".gitlab-ci.yml@Acme/Engineering-Assets:main"), &yes, &no, report.StatusOK, "read their CI file from main"}, + {"the source branch's CI file", str(""), &yes, &no, report.StatusFail, ".gitlab-ci.yml@acme/engineering-assets:main"}, + {"another project", str(".gitlab-ci.yml@acme/other:main"), &yes, &no, report.StatusFail, "source branch"}, + {"no pipeline needed", str(".gitlab-ci.yml@acme/engineering-assets:main"), &no, &no, report.StatusFail, "Pipelines must succeed"}, + {"skipped counts", str(".gitlab-ci.yml@acme/engineering-assets:main"), &yes, &yes, report.StatusFail, "skipped pipeline"}, + {"not shown", nil, nil, nil, report.StatusUnknown, "not shown"}, + } { + ks := hubch.KeyStore{Platform: "gitlab", RepoPath: "acme/engineering-assets", DefaultBranch: "main", + CIConfigPath: tc.path, MergeAfterPipeline: tc.after, MergeOnSkipped: tc.skipped} + lines := keyChecks(KeyLocationChecks(ks, nil, guard), "hub-guard") + if !hasCheck(lines, tc.status, tc.detail) { + t.Errorf("%s: no %s with %q in %q", tc.name, tc.status, tc.detail, lines) + } + if lines := keyChecks(KeyLocationChecks(ks, nil, hubWithIsolation("platform", "")), "hub-guard"); len(lines) != 0 { + t.Errorf("%s: hub-guard without guard: %q", tc.name, lines) + } + } +} diff --git a/internal/distribute/doctor_hub.go b/internal/distribute/doctor_hub.go index 4cfea7d..7d1ff9b 100644 --- a/internal/distribute/doctor_hub.go +++ b/internal/distribute/doctor_hub.go @@ -269,6 +269,9 @@ func KeyLocationChecks(ks hubch.KeyStore, known []string, cfg *config.Hub) []rep } if ks.Platform == "gitlab" { out = append(out, gitlabRefChecks(ks)...) + if cfg != nil && cfg.Security.WriterOnHub == "guard" { + out = append(out, gitlabGuardSettings(ks)) + } } for _, u := range ks.Unread { add(report.StatusUnknown, "not readable with this token: %s", u) @@ -580,6 +583,41 @@ func gitlabRefChecks(ks hubch.KeyStore) []report.DoctorCheck { return append(out, pv) } +// gitlabGuardSettings grades, under security.writer_on_hub guard, the hub's +// settings that make a merge wait for the plan of the default branch's CI +// configuration, as a maintainer's token reads them (check hub-guard; +// doctor's own hub-guard reads them as the writer, which GitLab may not +// show them to). +func gitlabGuardSettings(ks hubch.KeyStore) report.DoctorCheck { + c := report.DoctorCheck{Name: "hub-guard"} + var fails []string + unknown := false + switch { + case ks.CIConfigPath == nil: + unknown = true + case !config.PinnedCIConfig(*ks.CIConfigPath, ks.RepoPath, ks.DefaultBranch): + fails = append(fails, fmt.Sprintf("merge request pipelines read the CI file of their source branch (CI configuration file %q), which the writer "+ + "may rewrite to drop the plan that refuses its pushes: set it to .gitlab-ci.yml@%s:%s", *ks.CIConfigPath, ks.RepoPath, ks.DefaultBranch)) + } + switch { + case ks.MergeAfterPipeline == nil || ks.MergeOnSkipped == nil: + unknown = true + case !*ks.MergeAfterPipeline: + fails = append(fails, "a merge does not wait for the pipeline: turn on Pipelines must succeed") + case *ks.MergeOnSkipped: + fails = append(fails, "a skipped pipeline counts as a success: turn off Skipped pipelines are considered successful") + } + switch { + case len(fails) > 0: + c.Status, c.Detail = report.StatusFail, strings.Join(fails, "; ") + case unknown: + c.Status, c.Detail = report.StatusUnknown, "the hub's CI configuration path or merge checks are not shown to this token" + default: + c.Status, c.Detail = report.StatusOK, "the hub's pipelines read their CI file from "+ks.DefaultBranch+", and a merge waits for a pipeline that succeeded" + } + return c +} + // worse returns the worse of two statuses. func worse(a, b report.CheckStatus) report.CheckStatus { rank := map[report.CheckStatus]int{report.StatusOK: 0, report.StatusUnknown: 1, report.StatusWarn: 2, report.StatusFail: 3} diff --git a/internal/e2e/gitlab/guard_test.go b/internal/e2e/gitlab/guard_test.go new file mode 100644 index 0000000..7df0808 --- /dev/null +++ b/internal/e2e/gitlab/guard_test.go @@ -0,0 +1,171 @@ +//go:build e2e + +package gitlabe2e + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "net/http" + "os" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/bedrock-python/touchmark/internal/cli" + "github.com/bedrock-python/touchmark/internal/config" + "github.com/bedrock-python/touchmark/internal/platform/conformance" + "github.com/bedrock-python/touchmark/internal/report" +) + +// TestWriterOnHub runs security.writer_on_hub guard on GitLab: the hub is a +// project of a subgroup of the seeded group, which the writer is a +// Developer of, so the writer reaches the hub. The hub's default branch is +// protected (push No one, merge Maintainers), merges wait for a pipeline +// that succeeded and not a skipped one, and the CI file is read from the +// default branch (ci_config_path @:main). Its merge request +// pipelines run `touchmark plan` on the runner the harness registered. +// +// - doctor, with the writer's token, grades hub-guard ok; once the CI +// file is read from the source branch again, it fails; +// - the person's merge request: plan passes; +// - the writer pushes to the merge request's branch, with a CI file of +// its own that would drop plan: the pipeline still runs the default +// branch's CI file, and plan exits 2 naming the writer's push; +// - a merge request the writer opens: plan exits 2 naming the writer. +func TestWriterOnHub(t *testing.T) { + e := needLive(t) + if e.RunnerBin == "" { + t.Skip(envPrefix + "RUNNER_BIN is not set: the harness started no runner") + } + t.Setenv("CI", "") + fx := newGroup(t, e, "guard") + id := "e2e-" + randHex(t, 3) + root := e.api(e.Root) + hubYML := fmt.Sprintf("version: 1\nid: %s\nproviders:\n - id: gl\n type: gitlab\n url: %s\n writer: %s\n"+ + "security:\n writer_on_hub: guard\n", id, e.URL, e.Writer.Login) + ci := "workflow:\n rules:\n - if: $CI_PIPELINE_SOURCE == \"merge_request_event\"\nvariables:\n GIT_DEPTH: \"0\"\n" + + "plan:\n script:\n" + + " - git fetch --no-tags origin \"+refs/heads/$CI_DEFAULT_BRANCH:refs/remotes/origin/$CI_DEFAULT_BRANCH\"\n" + + fmt.Sprintf(" - '%s plan --hub .'\n", e.RunnerBin) + files := []conformance.File{ + {Path: config.HubFile, Content: []byte(hubYML)}, + {Path: config.TargetsFile, Content: []byte(fmt.Sprintf("version: 1\ndefaults:\n packs: [base]\ntargets:\n - group: %s\n", fx.ns))}, + {Path: "packs/base/AGENTS.md", Content: []byte(text("base AGENTS.md v1"))}, + {Path: ".gitlab-ci.yml", Content: []byte(ci)}, + } + hub := fx.createProject(t, fx.nsID, "hub", files) + e.waitAccess(t, hub.ID, e.Writer, e.Reader, e.Person) + pinned := ".gitlab-ci.yml@" + hub.PathWithNamespace + ":main" + + // The guard: main protected, the merge checks, the CI file of main. + root.do(t, http.MethodDelete, fmt.Sprintf("/projects/%d/protected_branches/main", hub.ID), nil) + root.ok(t, http.MethodPost, fmt.Sprintf("/projects/%d/protected_branches", hub.ID), + map[string]any{"name": "main", "push_access_level": 0, "merge_access_level": levelMaintainer, "allow_force_push": false}, nil) + root.ok(t, http.MethodPut, fmt.Sprintf("/projects/%d", hub.ID), map[string]any{ + "only_allow_merge_if_pipeline_succeeds": true, "allow_merge_on_skipped_pipeline": false, "ci_config_path": pinned, + }, nil) + root.ok(t, http.MethodPost, fmt.Sprintf("/projects/%d/variables", hub.ID), + map[string]any{"key": "TOUCHMARK_GL_READ_TOKEN", "value": e.Reader.Token, "masked": true, "protected": false}, nil) + + // doctor, as a maintainer runs it, from a clone of the hub. + dir := t.TempDir() + gitCmd(t, dir, nil, nil, "init", "-q", "-b", "main") + for _, f := range files { + full := filepath.Join(dir, filepath.FromSlash(f.Path)) + if err := os.MkdirAll(filepath.Dir(full), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(full, f.Content, 0o644); err != nil { + t.Fatal(err) + } + } + gitCmd(t, dir, nil, nil, "add", "-A") + gitCmd(t, dir, nil, nil, "commit", "-q", "-m", "the hub") + gitCmd(t, dir, nil, nil, "remote", "add", "origin", e.remote(hub.PathWithNamespace)) + fp := fmt.Sprintf("%s/%d", e.Host, hub.ID) + hubGuard := func(code int) report.DoctorCheck { + t.Helper() + t.Setenv("TOUCHMARK_GL_WRITE_TOKEN", e.Writer.Token) + args := []string{"doctor", "--hub", dir, "--hub-fp", fp, "--format", "json"} + var stdout, stderr bytes.Buffer + if got := cli.Main(context.Background(), args, &stdout, &stderr); got != code { + t.Fatalf("doctor: exit %d, want %d\n%s\n%s", got, code, e.Redact.Replace(stdout.String()), e.Redact.Replace(stderr.String())) + } + validate(t, "doctor", stdout.Bytes(), e.Redact.Replace) + var doc report.Doctor + if err := json.Unmarshal(stdout.Bytes(), &doc); err != nil { + t.Fatal(err) + } + for _, c := range doc.Providers[0].Checks { + if c.Name == "hub-hidden" { + t.Errorf("hub-hidden under guard: %+v", c) + } + if c.Name == "hub-guard" { + return c + } + } + t.Fatalf("no hub-guard in %+v", doc.Providers[0].Checks) + return report.DoctorCheck{} + } + if c := hubGuard(0); c.Status != report.StatusOK { + t.Errorf("hub-guard %s: %s", c.Status, c.Detail) + } + + repo := fx.repo(t, hub.ID) + // planJob is the plan job of the pipeline of merge request mr at its + // head now. + planJob := func(mr apiMR) ciJob { + t.Helper() + sha := e.mr(t, hub.ID, mr.IID).SHA + ref := fmt.Sprintf("refs/merge-requests/%d/head", mr.IID) + jobs := e.pipelineJobs(t, hub.ID, func() (int64, bool) { return e.pipelineOf(t, hub.ID, ref, sha) }) + job, ok := jobs["plan"] + if !ok { + t.Fatalf("the merge request pipeline ran %v, not the default branch's plan", keys(jobs)) + } + return job + } + + // The person's merge request: plan passes. + mr := fx.openMR(t, repo, e.Person, mrOptions{head: "feature", base: "main", title: "a change"}) + if job := planJob(mr); job.Status != "success" { + t.Fatalf("plan of the person's merge request: %s\n%s", job.Status, e.Redact.Replace(job.Trace)) + } + + // The writer pushes a CI file of its own to the branch: the pipeline + // still runs main's, whose plan refuses the push. + before := e.mr(t, hub.ID, mr.IID).SHA + e.commitFiles(t, e.Writer, hub.ID, "feature", "", []conformance.File{ + {Path: ".gitlab-ci.yml", Content: []byte("ok:\n script:\n - 'true'\n")}, + {Path: "packs/base/AGENTS.md", Content: []byte(text("the writer's AGENTS.md"))}, + }, "the writer's change") + e.waitMR(t, hub.ID, mr.IID, 2*time.Minute, func(m apiMR) bool { return m.SHA != before }) + job := planJob(mr) + if job.Status != "failed" || !strings.Contains(job.Trace, "pushed to feature") { + t.Errorf("plan after the writer's push: %s\n%s", job.Status, e.Redact.Replace(job.Trace)) + } + + // A merge request the writer opens. + wmr := fx.openMR(t, repo, e.Writer, mrOptions{head: "writer-change", base: "main", title: "the writer's"}) + if job := planJob(wmr); job.Status != "failed" || !strings.Contains(job.Trace, "was opened by") { + t.Errorf("plan of the writer's merge request: %s\n%s", job.Status, e.Redact.Replace(job.Trace)) + } + + // The CI file of the source branch again: hub-guard fails. + root.ok(t, http.MethodPut, fmt.Sprintf("/projects/%d", hub.ID), map[string]any{"ci_config_path": ""}, nil) + if c := hubGuard(1); c.Status != report.StatusFail || !strings.Contains(c.Detail, pinned) { + t.Errorf("hub-guard without the pin %s: %s", c.Status, c.Detail) + } +} + +// keys returns the names of jobs. +func keys(jobs map[string]ciJob) []string { + var out []string + for k := range jobs { + out = append(out, k) + } + return out +} diff --git a/internal/hubch/keys.go b/internal/hubch/keys.go index 7927355..ac4743b 100644 --- a/internal/hubch/keys.go +++ b/internal/hubch/keys.go @@ -76,6 +76,13 @@ type KeyStore struct { // (PipelineSettingsUnread says why). PipelineSettings *PipelineSettings PipelineSettingsUnread string + // CIConfigPath, MergeAfterPipeline and MergeOnSkipped are GitLab's + // ci_config_path, only_allow_merge_if_pipeline_succeeds and + // allow_merge_on_skipped_pipeline, for security.writer_on_hub guard; + // nil when not shown. + CIConfigPath *string + MergeAfterPipeline *bool + MergeOnSkipped *bool // Unread lists what could not be read, and why: the checks that need it // are unknown. Unread []string @@ -411,13 +418,17 @@ func (k *keyReader) gitlab(ctx context.Context) (KeyStore, error) { Kind string `json:"kind"` FullPath string `json:"full_path"` } `json:"namespace"` - PipelineVariables *string `json:"ci_pipeline_variables_minimum_override_role"` + PipelineVariables *string `json:"ci_pipeline_variables_minimum_override_role"` + CIConfigPath *string `json:"ci_config_path"` + MergeAfterPipeline *bool `json:"only_allow_merge_if_pipeline_succeeds"` + MergeOnSkipped *bool `json:"allow_merge_on_skipped_pipeline"` } id := k.in.RepoID if err := k.get(ctx, k.segs("projects", id), &project); err != nil { return KeyStore{}, fmt.Errorf("hub keys: read the hub project %s: %w", id, err) } - k.ks = KeyStore{Platform: "gitlab", RepoPath: project.Path, DefaultBranch: project.DefaultBranch, Visibility: visibility(project.Visibility)} + k.ks = KeyStore{Platform: "gitlab", RepoPath: project.Path, DefaultBranch: project.DefaultBranch, Visibility: visibility(project.Visibility), + CIConfigPath: project.CIConfigPath, MergeAfterPipeline: project.MergeAfterPipeline, MergeOnSkipped: project.MergeOnSkipped} if project.PipelineVariables != nil { k.ks.PipelineVariables = *project.PipelineVariables } else { diff --git a/internal/platform/gitlab/check.go b/internal/platform/gitlab/check.go index a70588e..de58134 100644 --- a/internal/platform/gitlab/check.go +++ b/internal/platform/gitlab/check.go @@ -50,10 +50,12 @@ type apiProtectedBranch struct { Name string `json:"name"` AllowForcePush bool `json:"allow_force_push"` Push []apiProtectedLevel `json:"push_access_levels"` + Merge []apiProtectedLevel `json:"merge_access_levels"` } -// apiProtectedLevel is who may push to a protected branch: a role (0 is -// No one, 30 Developers, 40 Maintainers), a user, a group or a deploy key. +// apiProtectedLevel is who may push or merge to a protected branch, or +// create a protected tag: a role (0 is No one, 30 Developers, 40 +// Maintainers), a user, a group or a deploy key. type apiProtectedLevel struct { AccessLevel int `json:"access_level"` UserID *int64 `json:"user_id"` diff --git a/internal/platform/gitlab/hubguard.go b/internal/platform/gitlab/hubguard.go new file mode 100644 index 0000000..8182753 --- /dev/null +++ b/internal/platform/gitlab/hubguard.go @@ -0,0 +1,250 @@ +package gitlab + +import ( + "context" + "fmt" + "strconv" + "strings" + + "github.com/bedrock-python/touchmark/internal/config" + "github.com/bedrock-python/touchmark/internal/platform" +) + +// A writer on the hub (security.writer_on_hub: guard): the writer, a +// Developer of a group that holds both the targets and the hub, reaches the +// hub. What keeps a leaked write key from changing what distribute ships +// is then the hub's own protection, which GuardHub verifies: +// +// - the writer is below Maintainer on the hub (a Maintainer changes the +// protection, the settings and the variables); +// - the default branch is protected, and none of the rules that match it +// lets the writer push or merge (GitLab applies the most permissive); +// - the hub's CI configuration is read from its default branch +// (ci_config_path "@:"): a merge +// request pipeline otherwise runs the CI file of the source branch, +// which the writer, pushing to it, could rewrite to drop the plan that +// refuses its pushes; +// - a merge waits for a pipeline that succeeded, and a skipped pipeline +// does not count (only_allow_merge_if_pipeline_succeeds, +// allow_merge_on_skipped_pipeline). +// +// Other protected branches the writer may push or merge to, and protected +// tags it may create, warn: their pipelines get the hub's protected +// variables. GitLab shows protected tags to Maintainers only, so the +// writer's view leaves them to doctor --hub-token (protected-tags). +// Developers and Reporters see the CI configuration path and the merge +// checks (checked on CE 18.11). Docs: https://docs.gitlab.com/api/protected_branches/, +// https://docs.gitlab.com/api/protected_tags/, +// https://docs.gitlab.com/api/projects/ (ci_config_path, merge settings), +// https://docs.gitlab.com/ci/pipelines/settings/#specify-a-custom-cicd-configuration-file. + +// apiHubSettings are the hub's settings GuardHub reads; nil when the +// answer leaves them out (they are shown to some roles only). +type apiHubSettings struct { + CIConfigPath *string `json:"ci_config_path"` + OnlyIfPipelineSucceed *bool `json:"only_allow_merge_if_pipeline_succeeds"` + MergeOnSkipped *bool `json:"allow_merge_on_skipped_pipeline"` +} + +// apiProtectedTag is one of GET /projects/:id/protected_tags. +type apiProtectedTag struct { + Name string `json:"name"` + Create []apiProtectedLevel `json:"create_access_levels"` +} + +// GuardHub reports the conditions of check "hub-guard" on hub +// (platform.HubGuard), as the writer reads them. +func (w *writer) GuardHub(ctx context.Context, hub platform.Repo) ([]platform.Finding, error) { + const op = "check the hub" + find := func(status platform.FindingStatus, format string, args ...any) platform.Finding { + return platform.Finding{Repo: hub.Path, Check: "hub-guard", Status: status, Detail: fmt.Sprintf(format, args...)} + } + unread := func(what string, err error) ([]platform.Finding, error) { + if stops(err) { + return nil, err + } + return []platform.Finding{find(platform.FindingUnknown, "cannot read %s: %v", what, err)}, nil + } + self, err := w.c.selfAccount(ctx) + if err != nil { + return nil, err + } + selfID, _ := strconv.ParseInt(self.ID, 10, 64) + id := projectID(hub) + p, err := w.c.getProject(ctx, op, id) + if err != nil { + return unread("the hub", err) + } + id = strconv.FormatInt(p.ID, 10) + branch := hub.DefaultBranch + if p.DefaultBranch != nil && *p.DefaultBranch != "" { + branch = *p.DefaultBranch + } + var out []platform.Finding + level, err := w.accessLevel(ctx, op, self, p) + switch { + case err != nil && stops(err): + return nil, err + case err != nil: + out = append(out, find(platform.FindingUnknown, "the writer's role on the hub cannot be read: %v", err)) + case level >= accessMaintainer: + out = append(out, find(platform.FindingFail, "the writer has access level %d on the hub: as a Maintainer it changes the hub's protection, "+ + "settings and variables; make it a Developer", level)) + default: + out = append(out, find(platform.FindingOK, "the writer has access level %d on the hub, below Maintainer", level)) + } + + rules, complete, err := w.listProtected(ctx, op, id) + if err != nil { + if stops(err) { + return nil, err + } + out = append(out, find(platform.FindingUnknown, "the hub's protected branches cannot be read: %v", err)) + } else { + out = append(out, defaultBranchGuard(find, rules, complete, branch, selfID, level)...) + out = append(out, otherBranchesGuard(find, rules, branch, selfID, level)...) + } + var tags []apiProtectedTag + _, err = listAll(ctx, w.c, op, w.c.projectURL(id, "protected_tags"), nil, maxProtectedPages, func(t apiProtectedTag) error { + tags = append(tags, t) + return nil + }) + switch c := platform.ClassOf(err); { + case err != nil && stops(err): + return nil, err + case c == platform.ClassPermission || c == platform.ClassNotFound: + // GitLab shows protected tags to Maintainers only (checked on CE + // 18.11: 403 to a Developer and a Reporter); doctor --hub-token + // grades them (protected-tags). + case err != nil: + out = append(out, find(platform.FindingUnknown, "the hub's protected tags cannot be read: %v", err)) + default: + for _, t := range tags { + switch canPush(t.Create, selfID, level) { + case platform.FindingOK: + out = append(out, find(platform.FindingWarn, "the writer may create the protected tag %s: its pipelines get the hub's protected variables", t.Name)) + case platform.FindingUnknown: + out = append(out, find(platform.FindingUnknown, "a group may create the protected tag %s, whose members the writer cannot read", t.Name)) + } + } + } + + var s apiHubSettings + if _, err := w.c.get(ctx, op, w.c.projectURL(id), nil, &s); err != nil { + if stops(err) { + return nil, err + } + return append(out, find(platform.FindingUnknown, "the hub's settings cannot be read: %v", err)), nil + } + return append(out, settingsGuard(find, s, p.PathWithNamespace, branch)...), nil +} + +// findFunc builds a Finding of hub-guard. +type findFunc func(status platform.FindingStatus, format string, args ...any) platform.Finding + +// defaultBranchGuard grades the protection of the default branch against +// the writer's pushes and merges. +func defaultBranchGuard(find findFunc, rules []apiProtectedBranch, complete bool, branch string, selfID int64, level int) []platform.Finding { + if branch == "" { + return []platform.Finding{find(platform.FindingUnknown, "the hub's default branch is unknown")} + } + v := verdictOf(rules, branch, selfID, level) + m := mergeVerdictOf(rules, branch, selfID, level) + names := strings.Join(v.rules, ", ") + var out []platform.Finding + switch { + case len(v.rules) == 0 && !complete: + return []platform.Finding{find(platform.FindingUnknown, "the hub has more protected branches than touchmark reads, and none read protects %s", branch)} + case len(v.rules) == 0: + return []platform.Finding{find(platform.FindingFail, "the default branch %s is not protected: the writer pushes to it; protect it with push No one and merge Maintainers", branch)} + } + switch v.push { + case platform.FindingOK: + out = append(out, find(platform.FindingFail, "protected branch %s lets the writer push to %s: allow No one to push", names, branch)) + case platform.FindingUnknown: + out = append(out, find(platform.FindingUnknown, "protected branch %s lets a group push to %s, whose members the writer cannot read", names, branch)) + default: + out = append(out, find(platform.FindingOK, "protected branch %s keeps the writer from pushing to %s", names, branch)) + } + switch m { + case platform.FindingOK: + out = append(out, find(platform.FindingFail, "protected branch %s lets the writer merge into %s: allow Maintainers alone to merge", names, branch)) + case platform.FindingUnknown: + out = append(out, find(platform.FindingUnknown, "protected branch %s lets a group merge into %s, whose members the writer cannot read", names, branch)) + default: + out = append(out, find(platform.FindingOK, "protected branch %s keeps the writer from merging into %s", names, branch)) + } + return out +} + +// mergeVerdictOf is verdictOf for merges: ok when a rule that matches +// branch lets the writer merge, unknown when none does but one lets a group +// merge, fail otherwise (and for a branch no rule matches). +func mergeVerdictOf(rules []apiProtectedBranch, branch string, selfID int64, level int) platform.FindingStatus { + status := platform.FindingFail + for _, rule := range rules { + if !protectedMatch(rule.Name, branch) { + continue + } + switch canPush(rule.Merge, selfID, level) { + case platform.FindingOK: + return platform.FindingOK + case platform.FindingUnknown: + status = platform.FindingUnknown + } + } + return status +} + +// otherBranchesGuard warns of the protected branches, besides those that +// match the default branch, the writer may push or merge to. +func otherBranchesGuard(find findFunc, rules []apiProtectedBranch, branch string, selfID int64, level int) []platform.Finding { + var out []platform.Finding + for _, rule := range rules { + if branch != "" && protectedMatch(rule.Name, branch) { + continue + } + push, merge := canPush(rule.Push, selfID, level), canPush(rule.Merge, selfID, level) + switch { + case push == platform.FindingOK || merge == platform.FindingOK: + out = append(out, find(platform.FindingWarn, "the writer may push or merge to the protected branch %s: its pipelines get the hub's protected variables", rule.Name)) + case push == platform.FindingUnknown || merge == platform.FindingUnknown: + out = append(out, find(platform.FindingUnknown, "a group may push or merge to the protected branch %s, whose members the writer cannot read", rule.Name)) + } + } + return out +} + +// settingsGuard grades the hub's settings that make a merge wait for the +// plan of the default branch's CI configuration. +func settingsGuard(find findFunc, s apiHubSettings, path, branch string) []platform.Finding { + var out []platform.Finding + switch { + case s.CIConfigPath == nil: + out = append(out, find(platform.FindingUnknown, "the hub's CI configuration path is not shown to the writer: doctor --hub-token reads it")) + case config.PinnedCIConfig(*s.CIConfigPath, path, branch): + out = append(out, find(platform.FindingOK, "the hub's pipelines read their CI file from the default branch %s (%s)", branch, *s.CIConfigPath)) + default: + out = append(out, find(platform.FindingFail, "the hub's merge request pipelines read the CI file of their source branch (CI configuration file %q), "+ + "which the writer may rewrite to drop the plan that refuses its pushes: set Settings > CI/CD > General pipelines > CI/CD "+ + "configuration file to .gitlab-ci.yml@%s:%s", strings.TrimSpace(deref(s.CIConfigPath)), path, branch)) + } + switch { + case s.OnlyIfPipelineSucceed == nil || s.MergeOnSkipped == nil: + out = append(out, find(platform.FindingUnknown, "the hub's merge checks are not shown to the writer: doctor --hub-token reads them")) + case !*s.OnlyIfPipelineSucceed: + out = append(out, find(platform.FindingFail, "a merge into the hub does not wait for its pipeline: turn on Settings > Merge requests > Pipelines must succeed")) + case *s.MergeOnSkipped: + out = append(out, find(platform.FindingFail, "a skipped pipeline counts as a success for a merge into the hub: turn off Skipped pipelines are considered successful")) + default: + out = append(out, find(platform.FindingOK, "a merge into the hub waits for a pipeline that succeeded, and a skipped one does not count")) + } + return out +} + +func deref(s *string) string { + if s == nil { + return "" + } + return *s +} diff --git a/internal/platform/gitlab/hubguard_test.go b/internal/platform/gitlab/hubguard_test.go new file mode 100644 index 0000000..5a1a92e --- /dev/null +++ b/internal/platform/gitlab/hubguard_test.go @@ -0,0 +1,144 @@ +package gitlab + +import ( + "net/http" + "strings" + "testing" + + "github.com/bedrock-python/touchmark/internal/platform" +) + +// guardLevel is one access level of a protected branch or tag: a role, or +// with user or group set, a user or a group. +func guardLevel(role int, user, group any) map[string]any { + return map[string]any{"id": role + 1, "access_level": role, "access_level_description": "x", "deploy_key_id": nil, "user_id": user, "group_id": group} +} + +// guardRule is a protected branch with push and merge levels. +func guardRule(name string, push, merge []any) map[string]any { + return map[string]any{"id": 1, "name": name, "allow_force_push": false, "push_access_levels": push, "merge_access_levels": merge} +} + +// guardHub serves the hub acme/engineering-assets (id 21, default branch +// main) to the writer (user 7, a Developer), with the protected branches +// rules, the protected tags tags, and the settings opts. +func guardHub(t *testing.T, role int, rules, tags []any, opts ...projectOpt) []platform.Finding { + t.Helper() + fx := newFixture(t) + fx.json(http.MethodGet, "/user", http.StatusOK, self(7, "group_9_bot_writer", true)) + opts = append([]projectOpt{with("permissions", map[string]any{"project_access": nil, + "group_access": map[string]any{"access_level": role, "notification_level": 3}})}, opts...) + fx.json(http.MethodGet, "/projects/21", http.StatusOK, project(21, "acme/engineering-assets", opts...)) + fx.pages("/projects/21/protected_branches", rules) + if tags == nil { + // What GitLab answers a Developer. + fx.json(http.MethodGet, "/projects/21/protected_tags", http.StatusForbidden, msg("403 Forbidden")) + } else { + fx.pages("/projects/21/protected_tags", tags) + } + var guard platform.HubGuard = fx.writer + fs, err := guard.GuardHub(t.Context(), platform.Repo{Host: fx.provider().Host, ID: "21", Path: "acme/engineering-assets", DefaultBranch: "main"}) + if err != nil { + t.Fatal(err) + } + for _, c := range fx.requests("", "") { + if c.Method != http.MethodGet { + t.Errorf("GuardHub wrote: %s %s", c.Method, c.Path) + } + } + return fs +} + +// guarded are the hub's settings of a well guarded hub. +var guarded = []projectOpt{ + with("ci_config_path", ".gitlab-ci.yml@acme/engineering-assets:main"), + with("only_allow_merge_if_pipeline_succeeds", true), + with("allow_merge_on_skipped_pipeline", false), +} + +// hasGuard reports whether fs has a finding of status whose detail holds +// sub. +func hasGuard(fs []platform.Finding, status platform.FindingStatus, sub string) bool { + for _, f := range fs { + if f.Check == "hub-guard" && f.Status == status && strings.Contains(f.Detail, sub) { + return true + } + } + return false +} + +func TestGuardHub(t *testing.T) { + maintainers := []any{guardLevel(40, nil, nil)} + noOne := []any{guardLevel(0, nil, nil)} + mainRule := guardRule("main", noOne, maintainers) + + t.Run("guarded", func(t *testing.T) { + fs := guardHub(t, 30, []any{mainRule}, []any{}, guarded...) + for _, f := range fs { + if f.Status != platform.FindingOK { + t.Errorf("%s: %s", f.Status, f.Detail) + } + } + if len(fs) != 5 { + t.Errorf("%d findings: %+v", len(fs), fs) + } + }) + // GitLab shows protected tags to Maintainers only: the writer's view + // leaves them out, doctor --hub-token grades them. + t.Run("protected tags not shown", func(t *testing.T) { + fs := guardHub(t, 30, []any{mainRule}, nil, guarded...) + for _, f := range fs { + if f.Status != platform.FindingOK { + t.Errorf("%s: %s", f.Status, f.Detail) + } + } + }) + for _, tc := range []struct { + name string + role int + rules []any + tags []any + opts []projectOpt + status platform.FindingStatus + want string + }{ + {"a Maintainer writer", 40, []any{mainRule}, []any{}, guarded, platform.FindingFail, "as a Maintainer it changes"}, + {"main unprotected", 30, []any{guardRule("release/*", noOne, maintainers)}, []any{}, guarded, platform.FindingFail, "main is not protected"}, + // GitLab applies the most permissive rule: a wildcard that lets + // Developers push wins over main's own. + {"a wildcard lets Developers push", 30, []any{mainRule, guardRule("ma*", []any{guardLevel(30, nil, nil)}, maintainers)}, []any{}, + guarded, platform.FindingFail, "lets the writer push to main"}, + {"the writer allowed by name", 30, []any{guardRule("main", []any{guardLevel(0, nil, nil), guardLevel(0, 7, nil)}, maintainers)}, []any{}, + guarded, platform.FindingFail, "lets the writer push to main"}, + {"Developers may merge", 30, []any{guardRule("main", noOne, []any{guardLevel(30, nil, nil)})}, []any{}, guarded, + platform.FindingFail, "lets the writer merge into main"}, + {"a group may merge", 30, []any{guardRule("main", noOne, []any{guardLevel(0, nil, 55)})}, []any{}, guarded, + platform.FindingUnknown, "lets a group merge into main"}, + {"a group may push", 30, []any{guardRule("main", []any{guardLevel(0, nil, 55)}, maintainers)}, []any{}, guarded, + platform.FindingUnknown, "lets a group push to main"}, + {"another protected branch", 30, []any{mainRule, guardRule("release", []any{guardLevel(30, nil, nil)}, maintainers)}, []any{}, guarded, + platform.FindingWarn, "protected branch release"}, + {"a protected tag the writer creates", 30, []any{mainRule}, + []any{map[string]any{"name": "v*", "create_access_levels": []any{guardLevel(30, nil, nil)}}}, guarded, platform.FindingWarn, "protected tag v*"}, + {"the CI file of the source branch", 30, []any{mainRule}, []any{}, + []projectOpt{with("ci_config_path", ""), with("only_allow_merge_if_pipeline_succeeds", true), with("allow_merge_on_skipped_pipeline", false)}, + platform.FindingFail, ".gitlab-ci.yml@acme/engineering-assets:main"}, + {"the CI file pinned to another branch", 30, []any{mainRule}, []any{}, + []projectOpt{with("ci_config_path", ".gitlab-ci.yml@acme/engineering-assets:dev"), with("only_allow_merge_if_pipeline_succeeds", true), + with("allow_merge_on_skipped_pipeline", false)}, platform.FindingFail, "read the CI file of their source branch"}, + {"a merge without the pipeline", 30, []any{mainRule}, []any{}, + []projectOpt{with("ci_config_path", ".gitlab-ci.yml@acme/engineering-assets:main"), with("only_allow_merge_if_pipeline_succeeds", false), + with("allow_merge_on_skipped_pipeline", false)}, platform.FindingFail, "Pipelines must succeed"}, + {"skipped pipelines count", 30, []any{mainRule}, []any{}, + []projectOpt{with("ci_config_path", ".gitlab-ci.yml@acme/engineering-assets:main"), with("only_allow_merge_if_pipeline_succeeds", true), + with("allow_merge_on_skipped_pipeline", true)}, platform.FindingFail, "Skipped pipelines"}, + {"settings not shown", 30, []any{mainRule}, []any{}, nil, platform.FindingUnknown, "not shown to the writer"}, + } { + t.Run(tc.name, func(t *testing.T) { + fs := guardHub(t, tc.role, tc.rules, tc.tags, tc.opts...) + if !hasGuard(fs, tc.status, tc.want) { + t.Errorf("no %s finding with %q in %+v", tc.status, tc.want, fs) + } + }) + } +} diff --git a/internal/platform/gitlab/mrguard.go b/internal/platform/gitlab/mrguard.go new file mode 100644 index 0000000..5eff5d7 --- /dev/null +++ b/internal/platform/gitlab/mrguard.go @@ -0,0 +1,91 @@ +package gitlab + +import ( + "context" + "errors" + "fmt" + "net/url" + "strconv" + + "github.com/bedrock-python/touchmark/internal/platform" +) + +// maxEventPages bounds the push events read looking for the creation of a +// merge request's source branch: 2 000. +const maxEventPages = 20 + +// apiMergeRequest is the part of GET /projects/:id/merge_requests/:iid the +// guard reads. +type apiMergeRequest struct { + IID int64 `json:"iid"` + SourceBranch string `json:"source_branch"` + SourceProjectID int64 `json:"source_project_id"` + Author *apiUser `json:"author"` +} + +// apiPushEvent is one of GET /projects/:id/events?action=pushed. +type apiPushEvent struct { + AuthorID int64 `json:"author_id"` + AuthorUsername string `json:"author_username"` + PushData *struct { + Action string `json:"action"` // pushed, created, removed + RefType string `json:"ref_type"` + Ref string `json:"ref"` + } `json:"push_data"` +} + +// errFoundCreation ends the listing of events once the source branch's +// creation is found. +var errFoundCreation = errors.New("found the branch's creation") + +// MergeRequestPushes reads merge request number of hub and who pushed to +// its source branch (platform.MergeRequestAuditor): GET +// /projects/:id/merge_requests/:iid, then the push events of the source +// project, newest first (GET /projects/:source/events?action=pushed), back +// to the event that created the branch. The events name who pushed, which a +// commit's author does not; a pipeline re-run by someone else does not hide +// them. Docs: https://docs.gitlab.com/api/merge_requests/, +// https://docs.gitlab.com/api/events/. Without the branch's creation among +// the events touchmark reads (more than it reads, events GitLab no longer +// keeps, a source project the reader cannot see), the answer is not +// Complete. +func (d *reader) MergeRequestPushes(ctx context.Context, hub platform.Repo, number int64) (platform.MergeRequestPushes, error) { + const op = "read the hub merge request" + var out platform.MergeRequestPushes + var mr apiMergeRequest + u := d.c.projectURL(projectID(hub), "merge_requests", strconv.FormatInt(number, 10)) + if _, err := d.c.get(ctx, op, u, nil, &mr); err != nil { + return out, err + } + if mr.IID != number || mr.SourceBranch == "" || mr.SourceProjectID <= 0 || mr.Author == nil || mr.Author.ID <= 0 { + return out, fmt.Errorf("gitlab: %s: the answer for !%d names no author, source branch or source project", op, number) + } + out.Author = platform.Account{ID: strconv.FormatInt(mr.Author.ID, 10), Login: mr.Author.Username} + out.Branch = mr.SourceBranch + seen := map[int64]bool{} + q := url.Values{"action": {"pushed"}, "sort": {"desc"}} + _, err := listAll(ctx, d.c, op, d.c.projectURL(strconv.FormatInt(mr.SourceProjectID, 10), "events"), q, maxEventPages, func(e apiPushEvent) error { + if e.PushData == nil || e.PushData.RefType != "branch" || e.PushData.Ref != mr.SourceBranch { + return nil + } + if e.AuthorID > 0 && !seen[e.AuthorID] { + seen[e.AuthorID] = true + out.Pushers = append(out.Pushers, platform.Account{ID: strconv.FormatInt(e.AuthorID, 10), Login: e.AuthorUsername}) + } + if e.PushData.Action == "created" { + return errFoundCreation + } + return nil + }) + switch { + case errors.Is(err, errFoundCreation): + out.Complete = true + case err != nil && stops(err): + return out, err + case err != nil: + out.Why = fmt.Sprintf("the push events of the source project cannot be read: %v", err) + default: + out.Why = fmt.Sprintf("the push events touchmark reads do not reach the creation of %s", mr.SourceBranch) + } + return out, nil +} diff --git a/internal/platform/gitlab/mrguard_test.go b/internal/platform/gitlab/mrguard_test.go new file mode 100644 index 0000000..7e774d9 --- /dev/null +++ b/internal/platform/gitlab/mrguard_test.go @@ -0,0 +1,81 @@ +package gitlab + +import ( + "net/http" + "testing" + + "github.com/bedrock-python/touchmark/internal/platform" +) + +// pushEvent is a push event of GET /projects/:id/events. +func pushEvent(author int64, username, action, ref string) map[string]any { + return map[string]any{ + "id": author*100 + int64(len(ref)), "project_id": 21, "action_name": "pushed to", "target_type": nil, "author_id": author, + "author_username": username, "created_at": "2026-10-09T10:00:00.000Z", + "push_data": map[string]any{"commit_count": 1, "action": action, "ref_type": "branch", "ref": ref, "commit_title": "x"}, + } +} + +func TestMergeRequestPushes(t *testing.T) { + audit := func(t *testing.T, events []any, mrOpts map[string]any) platform.MergeRequestPushes { + t.Helper() + fx := newFixture(t) + mr := map[string]any{"iid": 5, "source_branch": "feature", "source_project_id": 21, "target_branch": "main", + "author": basic(3, "jdoe")} + for k, v := range mrOpts { + mr[k] = v + } + fx.json(http.MethodGet, "/projects/21/merge_requests/5", http.StatusOK, mr) + if events != nil { + fx.pages("/projects/21/events", events) + } else { + fx.json(http.MethodGet, "/projects/99/events", http.StatusNotFound, msg("404 Project Not Found")) + } + var auditor platform.MergeRequestAuditor = fx.reader + got, err := auditor.MergeRequestPushes(t.Context(), platform.Repo{ID: "21", Path: "acme/engineering-assets"}, 5) + if err != nil { + t.Fatal(err) + } + for _, c := range fx.requests(http.MethodGet, "/projects/21/events") { + if c.Query.Get("action") != "pushed" || c.Query.Get("sort") != "desc" { + t.Errorf("events asked with %v", c.Query) + } + } + return got + } + logins := func(as []platform.Account) []string { + var out []string + for _, a := range as { + out = append(out, a.ID+":"+a.Login) + } + return out + } + + // Newest first: the writer pushed after the person created the branch; + // pushes to other branches and before the creation do not count. + got := audit(t, []any{ + pushEvent(9, "writer", "pushed", "other"), + pushEvent(7, "group_9_bot_writer", "pushed", "feature"), + pushEvent(3, "jdoe", "pushed", "feature"), + pushEvent(3, "jdoe", "created", "feature"), + pushEvent(8, "someone", "pushed", "feature"), + }, nil) + if !got.Complete || got.Author.ID != "3" || got.Branch != "feature" { + t.Errorf("answer %+v", got) + } + if l := logins(got.Pushers); len(l) != 2 || l[0] != "7:group_9_bot_writer" || l[1] != "3:jdoe" { + t.Errorf("pushers %q", l) + } + + // The creation is not among the events: not complete. + got = audit(t, []any{pushEvent(3, "jdoe", "pushed", "feature")}, nil) + if got.Complete || got.Why == "" { + t.Errorf("without the creation: %+v", got) + } + + // A source project the reader cannot see: not complete. + got = audit(t, nil, map[string]any{"source_project_id": 99}) + if got.Complete || got.Why == "" { + t.Errorf("an unseen fork: %+v", got) + } +} diff --git a/internal/platform/platform.go b/internal/platform/platform.go index 2745fba..e9316f0 100644 --- a/internal/platform/platform.go +++ b/internal/platform/platform.go @@ -497,6 +497,42 @@ type PushGuard interface { NoPush(ctx context.Context, r Repo, branches []string) ([]Protected, error) } +// HubGuard tells, for a writer that reaches the hub +// (security.writer_on_hub: guard), whether it can get content onto the +// hub's default branch. Optional (GitLab); doctor uses it. +type HubGuard interface { + // GuardHub returns the findings of check "hub-guard" on hub, one per + // condition: the writer's role, the protection of the default branch + // against its pushes and merges, the other protected branches and tags + // it may push or create, and the hub's settings that make a merge wait + // for the plan of the default branch's CI configuration. A condition it + // cannot read is FindingUnknown. Only a rate limit, a refused credential + // or the end of ctx fail the call. + GuardHub(ctx context.Context, hub Repo) ([]Finding, error) +} + +// MergeRequestAuditor tells who opened a merge request of the hub and who +// pushed to its source branch, for plan's guard under +// security.writer_on_hub guard. Optional (GitLab); the reader answers. +type MergeRequestAuditor interface { + // MergeRequestPushes reads merge request number of hub and the pushes + // to its source branch since the branch was created. + MergeRequestPushes(ctx context.Context, hub Repo, number int64) (MergeRequestPushes, error) +} + +// MergeRequestPushes is what MergeRequestAuditor found. +type MergeRequestPushes struct { + Author Account + // Branch is the source branch; Pushers the accounts that pushed to it, + // each once, since it was created. + Branch string + Pushers []Account + // Complete is false when the pushes could not be read back to the + // branch's creation; Why then says why. + Complete bool + Why string +} + // Protected is a branch a protection rule keeps the identity from pushing // to, and the rule (its name or pattern; empty when the platform does not // show it). diff --git a/internal/report/doctor.go b/internal/report/doctor.go index 9bb3a48..6e87b16 100644 --- a/internal/report/doctor.go +++ b/internal/report/doctor.go @@ -37,7 +37,9 @@ var Statuses = []CheckStatus{StatusOK, StatusWarn, StatusFail, StatusUnknown} // --hub-token); // - provider: writer (the write credential acts as hub.yml's writer), // token-expiry, scopes (a token's scopes), 2fa, hub-hidden (the writer -// cannot write to the hub), signing (whether pushes +// cannot write to the hub), hub-guard (under security.writer_on_hub +// guard: the writer reaches the hub but cannot get content onto its +// default branch), signing (whether pushes // that need a signature will get one), signing-key (the key is the // writer's on the platform); // - target: opt-in (the file could not be read), access, permissions, @@ -45,7 +47,7 @@ var Statuses = []CheckStatus{StatusOK, StatusWarn, StatusFail, StatusUnknown} // foreign fingerprint, the hub's markers by unknown authors). var DoctorChecks = []string{ "write-isolation", "environment", "key-location", "pipeline-variables", "protected-branches", "protected-tags", - "writer", "token-expiry", "scopes", "2fa", "hub-hidden", "signing", "signing-key", + "writer", "token-expiry", "scopes", "2fa", "hub-hidden", "hub-guard", "signing", "signing-key", "opt-in", "access", "permissions", "workflows", "rules", "markers", } diff --git a/internal/setup/gitlab.go b/internal/setup/gitlab.go index 94828a7..c078db5 100644 --- a/internal/setup/gitlab.go +++ b/internal/setup/gitlab.go @@ -62,6 +62,11 @@ type GitLabInput struct { Client *httpx.Client // Isolation is security.write_isolation: platform or external. Isolation string + // WriterOnHub is security.writer_on_hub: refuse (or "") keeps the hub + // outside the group of targets; guard lets the writer reach the hub as + // a Developer, and sets up and checks what then keeps it from getting + // content onto the hub's default branch. + WriterOnHub string // Accounts is AccountsAuto (service accounts where the instance lets // the token create them, else group access tokens), AccountsService or // AccountsGroup. @@ -149,6 +154,10 @@ type glProject struct { PipelineVariablesRole *string `json:"ci_pipeline_variables_minimum_override_role"` RestrictVariables *bool `json:"restrict_user_defined_variables"` ProtectMRPipelines *bool `json:"protect_merge_request_pipelines"` + // The settings of security.writer_on_hub guard. + CIConfigPath *string `json:"ci_config_path"` + MergeAfterPipeline *bool `json:"only_allow_merge_if_pipeline_succeeds"` + MergeOnSkipped *bool `json:"allow_merge_on_skipped_pipeline"` } // glGroup is a group. @@ -240,6 +249,9 @@ type gitlabSetup struct { reader *glAccount writer *glAccount variabs []glVariable + // inside is set when the hub is inside the group of targets + // (security.writer_on_hub guard only). + inside bool } // GitLab sets up the hub's GitLab for touchmark (see the package @@ -303,7 +315,11 @@ func GitLab(ctx context.Context, in GitLabInput) (*Report, error) { g.role(ctx, g.writer) } r.Reader, r.Writer = g.reader.login, g.writer.login - g.writerHidden(ctx) + if g.guard() { + g.writerGuard(ctx) + } else { + g.writerHidden(ctx) + } g.environment(ctx) if err := g.readVariables(ctx); err != nil { r.add("variables", StatusFail, "cannot list the hub's CI/CD variables: %v", err) @@ -334,7 +350,7 @@ func id(n int64) string { return strconv.FormatInt(n, 10) } // identify reads who the token is, the hub and the group, and checks that // setup may go on: a Maintainer of the hub, an Owner of the group, and the -// hub outside the group. +// hub outside the group unless security.writer_on_hub is guard. func (g *gitlabSetup) identify(ctx context.Context) error { in := g.in if err := g.get(ctx, &g.me, "user"); err != nil { @@ -381,9 +397,11 @@ func (g *gitlabSetup) identify(ctx context.Context) error { } g.r.Group = g.group.FullPath ns, gp := strings.ToLower(g.hub.Namespace.FullPath), strings.ToLower(g.group.FullPath) - if ns == gp || strings.HasPrefix(ns, gp+"/") { + g.inside = ns == gp || strings.HasPrefix(ns, gp+"/") + if g.inside && !g.guard() { return precondition("the hub %s is inside the group of targets %s: the writer, a member of that group, would reach the hub "+ - "and could change the packs with a leaked key; keep the hub in a group outside it", g.hub.Path, g.group.FullPath) + "and could change the packs with a leaked key; keep the hub in a group outside it, or set security.writer_on_hub: guard "+ + "in hub.yml so that setup checks the writer cannot get content onto the hub's default branch", g.hub.Path, g.group.FullPath) } if !g.me.IsAdmin { var m struct { @@ -418,10 +436,17 @@ func (g *gitlabSetup) identify(ctx context.Context) error { version = fmt.Sprintf("GitLab %d.%d, ", g.major, g.minor) } g.r.add("hub", StatusOK, "%s%s, default branch %s; %s is %s", version, g.hub.Path, g.hub.DefaultBranch, g.me.Username, who) - g.r.add("group", StatusOK, "%s holds the targets; the hub is outside it", g.group.FullPath) + if g.inside { + g.r.add("group", StatusOK, "%s holds the targets and the hub (security.writer_on_hub: guard)", g.group.FullPath) + } else { + g.r.add("group", StatusOK, "%s holds the targets; the hub is outside it", g.group.FullPath) + } return nil } +// guard reports whether the run sets up security.writer_on_hub guard. +func (g *gitlabSetup) guard() bool { return g.in.WriterOnHub == "guard" } + // chooseAccounts decides what the reader and the writer are: what the run // was told; else group access tokens when setup's tokens already exist in // the group; else service accounts when the instance lists them; else @@ -748,6 +773,37 @@ func (g *gitlabSetup) writerHidden(ctx context.Context) { } } +// writerGuard checks, under security.writer_on_hub guard, that the +// writer's role on the hub is below Maintainer: a Maintainer changes the +// hub's protection, settings and variables. The rest of the guard is what +// setup sets up after it (defaultBranch, projectSettings) and checks +// (verify). +func (g *gitlabSetup) writerGuard(ctx context.Context) { + w := g.writer + switch { + case w.skip: + return + case w.id == 0: + g.r.add("writer-guard", StatusWould, "would check that the writer is below Maintainer on the hub") + return + } + var m struct { + Level int `json:"access_level"` + } + err := g.get(ctx, &m, "projects", id(g.hub.ID), "members", "all", id(w.id)) + switch { + case isStatus(err, http.StatusNotFound): + g.r.add("writer-guard", StatusOK, "%s is no member of the hub", w.login) + case err != nil: + g.r.add("writer-guard", StatusUnknown, "cannot read the role of %s on the hub: %v", w.login, err) + case m.Level >= glMaintainer: + g.r.add("writer-guard", StatusFail, "%s reaches the hub as %s, and a Maintainer changes the hub's protection, settings and variables: "+ + "give it the Developer role", w.login, levelName(m.Level)) + default: + g.r.add("writer-guard", StatusOK, "%s reaches the hub as %s, below Maintainer (security.writer_on_hub: guard)", w.login, levelName(m.Level)) + } +} + // environment creates the environment touchmark-distribute. Docs: // https://docs.gitlab.com/api/environments/#create-an-environment func (g *gitlabSetup) environment(ctx context.Context) { @@ -1256,6 +1312,9 @@ func (g *gitlabSetup) projectSettings(ctx context.Context) { } did = append(did, "pipeline-variables") } + if g.guard() { + did = append(did, g.guardSettings(put)...) + } switch { case h.ProtectMRPipelines != nil && *h.ProtectMRPipelines: put["protect_merge_request_pipelines"] = false @@ -1274,6 +1333,8 @@ func (g *gitlabSetup) projectSettings(ctx context.Context) { detail := map[string]string{ "pipeline-variables": "the minimum role for pipeline variables is No one allowed", "mr-pipelines": "merge request pipelines get no protected variables", + "merge-checks": "a merge waits for a pipeline that succeeded, and a skipped pipeline does not count", + "ci-config": "the pipelines read their CI file from the default branch: " + g.pinnedCIConfig(), } if g.r.DryRun { for _, d := range did { @@ -1291,12 +1352,69 @@ func (g *gitlabSetup) projectSettings(ctx context.Context) { g.r.add(d, StatusFail, "GitLab kept the minimum role for pipeline variables: set it to No one allowed in Settings > CI/CD > Variables") case d == "mr-pipelines" && (after.ProtectMRPipelines == nil || *after.ProtectMRPipelines): g.r.add(d, StatusFail, "GitLab still lets merge request pipelines read protected variables: turn it off in Settings > CI/CD > Variables") + case d == "merge-checks" && (after.MergeAfterPipeline == nil || !*after.MergeAfterPipeline || after.MergeOnSkipped == nil || *after.MergeOnSkipped): + g.r.add(d, StatusFail, "GitLab kept the merge checks: in Settings > Merge requests turn on Pipelines must succeed and turn off "+ + "Skipped pipelines are considered successful") + case d == "ci-config" && (after.CIConfigPath == nil || *after.CIConfigPath != g.pinnedCIConfig()): + g.r.add(d, StatusFail, "GitLab kept the CI configuration path: set Settings > CI/CD > General pipelines > CI/CD configuration file "+ + "to %s", g.pinnedCIConfig()) default: g.r.add(d, StatusDone, "%s", detail[d]) } } } +// guardSettings adds to put, under security.writer_on_hub guard, the +// hub's settings that make a merge wait for the plan of the default +// branch's CI configuration: Pipelines must succeed, no merge on a skipped +// pipeline, and the CI file read from the default branch +// (ci_config_path @:), so that a merge request +// pipeline runs the reviewed configuration, whose plan refuses a merge +// request the writer opened or pushed to. It reports the settings already +// so, and returns the steps whose settings it put. +func (g *gitlabSetup) guardSettings(put map[string]any) []string { + h := g.hub + var did []string + switch { + case h.MergeAfterPipeline == nil || h.MergeOnSkipped == nil: + g.r.add("merge-checks", StatusManual, "this GitLab does not show the merge checks to the token: in Settings > Merge requests turn on "+ + "Pipelines must succeed and turn off Skipped pipelines are considered successful") + case *h.MergeAfterPipeline && !*h.MergeOnSkipped: + g.r.add("merge-checks", StatusOK, "a merge waits for a pipeline that succeeded, and a skipped pipeline does not count") + default: + put["only_allow_merge_if_pipeline_succeeds"] = true + put["allow_merge_on_skipped_pipeline"] = false + did = append(did, "merge-checks") + } + pinned := g.pinnedCIConfig() + switch { + case h.CIConfigPath == nil: + g.r.add("ci-config", StatusManual, "this GitLab does not show the CI configuration path to the token: set Settings > CI/CD > "+ + "General pipelines > CI/CD configuration file to %s", pinned) + case config.PinnedCIConfig(*h.CIConfigPath, h.Path, h.DefaultBranch): + g.r.add("ci-config", StatusOK, "the pipelines read their CI file from the default branch: %s", *h.CIConfigPath) + default: + put["ci_config_path"] = pinned + did = append(did, "ci-config") + } + return did +} + +// pinnedCIConfig is the hub's CI configuration path that reads its CI file +// from the default branch: the file the hub reads now (.gitlab-ci.yml by +// default), @ the hub's path, : the default branch. +func (g *gitlabSetup) pinnedCIConfig() string { + file := ".gitlab-ci.yml" + if p := g.hub.CIConfigPath; p != nil && *p != "" && !strings.ContainsAny(*p, "@:") { + file = *p + } else if p != nil && *p != "" { + if f, _, ok := strings.Cut(*p, "@"); ok && f != "" && !strings.Contains(f, ":") { + file = f + } + } + return file + "@" + g.hub.Path + ":" + g.hub.DefaultBranch +} + // schedules creates the daily distribute and weekly doctor schedules of // the default branch, owned by the token's user. Docs: // https://docs.gitlab.com/api/pipeline_schedules/ @@ -1347,7 +1465,8 @@ func (g *gitlabSetup) verify(ctx context.Context) { g.r.add("check", StatusUnknown, "cannot read where the hub keeps its keys: %v", err) return } - addChecks(g.r, distribute.KeyLocationChecks(ks, []string{g.in.WriteVar}, &config.Hub{Security: config.Security{WriteIsolation: g.in.Isolation}})) + addChecks(g.r, distribute.KeyLocationChecks(ks, []string{g.in.WriteVar}, &config.Hub{Security: config.Security{WriteIsolation: g.in.Isolation, + WriterOnHub: g.in.WriterOnHub}})) } // addChecks adds doctor's checks as steps "check ". diff --git a/internal/setup/gitlab_test.go b/internal/setup/gitlab_test.go index 9d09ffd..55197af 100644 --- a/internal/setup/gitlab_test.go +++ b/internal/setup/gitlab_test.go @@ -654,3 +654,58 @@ func TestDefaultNames(t *testing.T) { t.Errorf("%d characters", len(r)) } } + +// TestGitLabWriterOnHub: under security.writer_on_hub guard a hub inside +// the group of targets is set up: the writer reaches it as a Developer, +// merges wait for a pipeline that succeeded, and the CI file is read from +// the default branch; a writer that is a Maintainer fails. +func TestGitLabWriterOnHub(t *testing.T) { + w := newGLWorld(t, "18.11") + inner := w.f.addProject("hub2", w.services.id, nil) + in := w.input(false) + in.Project, in.WriterOnHub = "acme/services/hub2", "guard" + rep := w.run(t, in) + wantSteps(t, rep, map[string]Status{"group": StatusOK, "writer-guard": StatusOK, "merge-checks": StatusDone, "ci-config": StatusDone}) + if d := step(t, rep, "group").Detail; !strings.Contains(d, "holds the targets and the hub") { + t.Errorf("group %q", d) + } + if d := step(t, rep, "writer-guard").Detail; !strings.Contains(d, "Developer") { + t.Errorf("writer-guard %q", d) + } + w.f.mu.Lock() + settings := inner.settings + w.f.mu.Unlock() + if settings["ci_config_path"] != ".gitlab-ci.yml@acme/services/hub2:main" || settings["only_allow_merge_if_pipeline_succeeds"] != true || + settings["allow_merge_on_skipped_pipeline"] != false { + t.Errorf("settings %v", settings) + } + if s := step(t, rep, "check hub-guard"); s.Status != StatusOK { + t.Errorf("check hub-guard %s %q", s.Status, s.Detail) + } + // A second run changes nothing. + rep = w.run(t, in) + wantSteps(t, rep, map[string]Status{"merge-checks": StatusOK, "ci-config": StatusOK}) + + // A custom CI file keeps its name. + w.f.mu.Lock() + inner.settings["ci_config_path"] = "ci/hub.yml" + w.f.mu.Unlock() + w.run(t, in) + w.f.mu.Lock() + got := inner.settings["ci_config_path"] + w.f.mu.Unlock() + if got != "ci/hub.yml@acme/services/hub2:main" { + t.Errorf("ci_config_path %v", got) + } + + // A writer that is a Maintainer of the hub fails. + for _, u := range w.f.users { + if u.username == rep.Writer { + inner.members[u.id] = 40 + } + } + rep = w.run(t, in) + if s := step(t, rep, "writer-guard"); s.Status != StatusFail || rep.ExitCode() != ExitFailed { + t.Errorf("writer-guard %s %q, exit %d", s.Status, s.Detail, rep.ExitCode()) + } +} diff --git a/internal/setup/glfake_test.go b/internal/setup/glfake_test.go index aeffb7a..dd509b4 100644 --- a/internal/setup/glfake_test.go +++ b/internal/setup/glfake_test.go @@ -34,7 +34,9 @@ import ( // (premium); on Free it changes allow_force_push alone; // - projects show ci_pipeline_variables_minimum_override_role (and the // deprecated restrict_user_defined_variables) to Maintainers, and -// protect_merge_request_pipelines from 18.10. +// protect_merge_request_pipelines from 18.10; ci_config_path, +// only_allow_merge_if_pipeline_succeeds and +// allow_merge_on_skipped_pipeline. type glFake struct { t *testing.T srv *httptest.Server @@ -175,6 +177,10 @@ func (f *glFake) addProject(name string, ns int64, members map[int64]int) *fProj p.branches["main"] = &fProtected{push: []fLevel{{id: f.id(), level: 40}}, merge: []fLevel{{id: f.id(), level: 40}}} p.settings["ci_pipeline_variables_minimum_override_role"] = "maintainer" p.settings["restrict_user_defined_variables"] = false + // As GitLab shows a new project's (CE 18.11, checked live). + p.settings["ci_config_path"] = "" + p.settings["only_allow_merge_if_pipeline_succeeds"] = false + p.settings["allow_merge_on_skipped_pipeline"] = false if f.atLeast(18, 10) { p.settings["protect_merge_request_pipelines"] = true } diff --git a/schemas/hub.schema.json b/schemas/hub.schema.json index 7ec8b2f..5381f43 100644 --- a/schemas/hub.schema.json +++ b/schemas/hub.schema.json @@ -146,6 +146,10 @@ "private_targets_in_public_hub": { "description": "What a public hub does with non-public targets: skip them, or deliver and let their names reach public CI logs. Default: skip.", "enum": ["skip", "deliver"] + }, + "writer_on_hub": { + "description": "What a writer that reaches the hub means. refuse: doctor fails it, and setup gitlab refuses a hub inside the group of targets. guard (GitLab): the writer may be a Developer of the hub; doctor verifies that it cannot get content onto the default branch, and plan refuses a hub merge request the writer opened or pushed to. Default: refuse.", + "enum": ["refuse", "guard"] } }, "if": {