ci(mirror): fail-closed publish guard + publisher for the public deliverable mirror - #1034
Conversation
…verable mirror The public face of this repo becomes a separate, deliverable-only mirror (charts, installer, README/LICENSE/docs, the Pages chart index, releases). Nothing reaches it except through the new Mirror publish workflow, and nothing leaves that workflow except what scripts/publish-guard.sh staged from the explicit allowlist (.publish-include) and cleared through four guards: forbidden paths and forbidden strings (.publish-forbidden), and gitleaks. Every guard fails closed; "could not tell" never publishes. scripts/publish-mirror.sh does the push: it refuses an unset mirror and a mirror equal to this repository, pushes plainly (never force), and never overwrites an existing release. The workflow runs after Release Helm Chart completes and on dispatch with dry-run defaulting to true; the mirror name (MIRROR_REPO) has no default. Customer identifiers for the string scan are supplied privately at publish time, not committed to this public file. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
bugbot run |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
A guard that can never pass is a guard nobody arms: on the real tree the
single [strings] tier refused with 825 hits, all of them internal ticket
identifiers and non-production hostnames whose fate is still a decision,
not a defect. The scan now has two tiers in .publish-forbidden:
- [strings-refuse] mailboxes (support@ spared), arn:aws:, ECR account ids,
plus the private tenant needles from --extra-forbidden.
A hit refuses, as before.
- [strings-report] ticket/RFC identifiers and non-production hosts. Hits are
counted and printed (per-needle totals, ten most-hit
files) but refuse only under --strict.
The list itself is refused (exit 2) on an unknown section header, a needle
in both tiers, or an empty [strings-refuse]. The header regex is loose on
purpose so a misspelt header is refused by name rather than read as a
needle of the section before it.
The workflow gains a `strict` dispatch input (default false) and honours the
PUBLISH_STRICT repository variable for every run, including workflow_run,
which has no inputs; flipping either arms the report tier.
Tests: 39 -> 47. Report-tier hit alone -> exit 0 with count and table; the
same under --strict -> exit 1 naming [strings-report (strict)]; refuse-tier
hit -> exit 1 naming [strings-refuse]; duplicate needle, unknown section
and empty refuse tier -> exit 2 with the offender named. The committed list
is exercised needle by needle with inputs written in the test, and the real
tree is now asserted clean (exit 0). Each new check was mutation-proved.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
bugbot run |
…keep prereleases off the default branch Review findings on the mirror-publish workflow, each with its test: - Untrusted checkout. The job checked out the release tag and then ran scripts/publish-guard.sh and publish-mirror.sh from it while the job went on to mint an App token. Now the one actions/checkout has no ref (the workflow's own commit), and the release tag is fetched separately into a detached worktree under RUNNER_TEMP as data — staged and scanned, never executed. Before the fetch the plan step requires the release object's tag_name to equal the run's tag and takes the expected commit from workflow_run.head_sha (a dispatch asks the API); the fetched tag must resolve to exactly that commit or the run refuses. The guard reads the allowlist and forbidden list from the tooling checkout, whatever the tag carries. - Prerelease overwrote the mirror's default branch. workflow_run always publishes, and the tree push never looked at PRERELEASE. The plan step now derives publish_tree=false for a prerelease and says why; the default branch and gh-pages pushes are gated on it; the release is still created, marked prerelease, pinned to the mirror's current default-branch head — an empty mirror is refused rather than given an RC as its first content. - Captured output hid refusals. `target` and `tree` ran through `$(...)`, so under set -e their ::error:: lines never reached the log. Both now run directly and write their results (repo=/name=, result=/sha=) through a new --output FILE option, which the workflow points at $GITHUB_OUTPUT. - The gitleaks download carries --tlsv1.2 like every other privileged fetch. scripts/tests/mirror-publish-workflow.bats executes the plan, src, target and keep step bodies read out of the workflow itself (gh shimmed, the tag fetch against a real bare repo) and pins the shape — no checkout ref, every tree push gated, the release step not, no captured publisher, one pinned tag fetch — with mutations asserted to change the document before they are judged. publish-mirror.bats covers --output for target and tree, including that a refusal writes nothing and annotates stdout. Each new check was mutation-proved against the real workflow and script. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
bugbot run |
shujaatTracebloc
left a comment
shujaatTracebloc
left a comment
There was a problem hiding this comment.
Reviewed at 03ec0ab (I had a review drafted against 92399d8; 03ec0ab landed while I was writing it, so this is against the new head).
What 03ec0ab fixed — verified, not just read from the thread replies
- Untrusted checkout (CodeQL): the only
actions/checkoutnow takes noref; the tag is fetched with--no-tags --depth 1into a detached worktree under$RUNNER_TEMPand refused unlessFETCH_HEAD^{commit}equalsworkflow_run.head_sha(or the API's commit for a dispatch), and the release'stagNamemust equal the run's tag. The guard, publisher and both policy lists run fromgithub.sha. That is the structural fix, and it also means the current guard always runs rather than the one frozen in the tag. - Prerelease overwrote the mirror:
publish_tree=falseforisPrerelease, bothtreepushes gated on it, release still mirrored--prereleaseand pinned to the mirror's default-branch head via the newkeepstep, which refuses on an empty mirror instead of inventing a target. Correct. $(…)swallowed refusals:publish-mirror.sh target|tree --output FILEwritesrepo=/name=andresult=/sha=to$GITHUB_OUTPUT, nothing on a refusal, unwritable file is could-not-tell; both call sites run the script directly. Correct.- gitleaks
curlhas--tlsv1.2.
Still blocking
-
Tenant needles are printed to a public log —
scripts/publish-guard.shis byte-identical on 92399d8 and 03ec0ab (blobff7349e), so Bugbot's open thread at:394stands, and it is a real disclosure, not a style point:refuse/noteat:390/:394(andGREP_ERRat:358/:370) interpolate the needle pattern into stdout, the guard step tees that into$GITHUB_STEP_SUMMARY(mirror-publish.yaml, guard steps), and tracebloc/client is public. The--extra-forbiddenneedles are exactly the customer identifiers kept out of.publish-forbiddenbecause the file is public; one hit prints them on a public run page..publish-forbidden's own contract says "names the file and line (never the matched text)". Fix: private needles print as a tier + ordinal ([strings-refuse] private needle #3) or a short hash, never the pattern; committed-tier needles may keep their name.publish-guard.bats:1355-1364currently assertsneedle 'arn:aws:' found …is echoed while asserting the matched text is not — split that assertion the same way, and add one that plants an extra-forbidden needle and asserts the pattern is absent from$output. Same file, same fix on tracebloc/cli#654. -
CI is not finished on this head —
Unit tests,mutation-check,bugbot / reviewand Bugbot itself are pending. Worth knowing before they land: the cli twin's new workflow harness (mirror-publish-workflow-verify.sh, run in cli'sInstaller (shell)job) is red on a0d4231 withsrc fetch (rc=1): … the release was cut at HEAD— thesrcfixture passes the literal stringHEADasEXPECT_SHAin a directory that is not a repo (fatal: ambiguous argument 'HEAD'). Ifscripts/tests/mirror-publish-workflow.batshere shares that fixture,Unit testswill go red the same way; resolve the sha in the fixture before feeding it to the step body.
I will approve once the guard stops printing private needles and the head is green with Bugbot clean.
…s on a fresh runner The workflow test's bare origin relied on init.defaultBranch: unset on the runner, its HEAD pointed at a `master` nobody pushed, the clone had an unborn HEAD, and `rev-parse HEAD` handed the src step the literal word HEAD as the expected commit — the case failed with "cut at HEAD" instead of proving the pinned fetch. The bare HEAD is now set to main explicitly and the commit is read from the seed repository; reproduced locally with GIT_CONFIG_KEY_0=init.defaultBranch GIT_CONFIG_VALUE_0=master before and after. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
Thanks — both blockers accepted.
|
… by number only Two review findings on scripts/publish-guard.sh (kept byte-identical with the cli repository's copy): - The [allow] pass removed a token with an unanchored, case-sensitive sed replace before re-testing the needle. A mailbox that merely ENDS in the public support address (devsupport@…) lost the token, the mailbox rule no longer matched, and the internal address could ship; a differently-cased public address was refused. The strip now matches the token as a whole word — not the tail of a longer local part, not the head of a longer domain, a sentence-ending dot still a boundary — and case-insensitively, as the scan itself matches. - Refuse-tier hits printed the needle, and the workflow tees that log into the run summary. The needles from --extra-forbidden are the identifiers kept out of the committed list because this repository is public, so one deliverable hit would have published them in the log. Private needles are now scanned in their own pass and named `private needle #N` in every line the guard prints or writes (stdout, the report file, grep-error text); the committed needles are still named by pattern. publish-guard.bats: devsupport@ is refused, Support@Tracebloc.io. at a sentence end passes, and the tenant case asserts the redacted name and that the pattern appears neither in the output nor in publish-guard-report.txt. Each check was mutation-proved by restoring the old strip and the old label in the script and watching only its named cases redden. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ult branch A re-run of the chart workflow for an earlier tag, or a dispatch naming one, used to push that tag's tree to the mirror's default branch and chart index, rolling the public README, installer and charts back — and, when that release was already mirrored, stopping there with the rollback in place. The plan step now asks GitHub for the source repository's releases/latest and sets publish_tree=false for any stable tag that is not it; the release is still mirrored, pinned to the mirror's current default-branch head. An unreadable releases/latest refuses the run rather than guessing. Tests (mirror-publish-workflow.bats, executing the real step bodies): the newest stable release publishes the tree and asks releases/latest of the source repo; an older stable tag sets publish_tree=false with a notice and is not marked prerelease; an unreadable releases/latest is refused; a mutation that removes the comparison lets the older tag publish the tree (so the test is live). The shape check now derives the step that decides publish_tree and refuses one that never reads releases/latest, mutation-proved. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
bugbot run |
|
@shujaatTracebloc both points from your review are on the head now:
1fb3ea9 adds one more fix, pre-empting a Bugbot finding filed on the cli twin: a re-run of the chart workflow for an older stable tag (or a dispatch naming one) used to push that tag's tree to the mirror's default branch and chart index, rolling the public deliverable back. The plan step now reads the source repo's Local: 2061/2061 bats, |
… under errexit Two review findings on the workflow body. isPrerelease was compared only against an exact `true`, so a release object missing the field (or carrying anything else) read as stable and left the default-branch and gh-pages pushes armed. The plan step now accepts exactly `true` or `false` and refuses everything else. Actions runs every `run:` body with -e, which `set -uo pipefail` does not turn off, so a refusing guard exited the step at the tee pipeline before the step summary was written or `exit "$rc"` ran. Both guard steps now catch the guard's status with `|| rc=$?`, write the summary, and exit with that status. The steps gained ids (guard-tree, guard-pages) so their bodies are testable. Tests (mirror-publish-workflow.bats): step bodies now run under `bash -e`, as in CI; a release JSON without isPrerelease is refused; the guard-tree body with a planted refusing guard writes the summary and exits 1; the mutation that drops `|| rc=$?` reproduces the finding (exit 1, empty summary). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 37600f4. Configure here.
saqlainsyed007
left a comment
saqlainsyed007
left a comment
There was a problem hiding this comment.
Not approving — @shujaatTracebloc holds a standing CHANGES_REQUESTED (at 03ec0ab), and it is his to clear.
For what it's worth, both conditions you named look satisfied on the current head 37600f4:
- Private-needle disclosure is fixed.
scripts/publish-guard.shno longer interpolates an extra-forbidden pattern into any message: private needles are namedprivate needle #Nvia$shown, and every refuse/note/report path uses$shown, never$needle, for that tier. The exact test you asked for is inpublish-guard.bats("mutation: a private needle supplied with --extra-forbidden joins the refuse tier, named by number only"): it plantsplanted-tenant, asserts the refusal reads[strings-refuse] private needle #1 …, and asserts the pattern is absent from both$outputandpublish-guard-report.txt. - Head is green (Unit tests, mutation-check, bugbot/review all pass) and Cursor Bugbot is clean on
37600f4.
Independent correctness pass on the guard found no defect — it fails closed (exit 2 on unreadable/empty/malformed lists, zero tracked files, allowlist matching nothing, missing/crashed scanner), needle_hits routes through a file rather than producer | grep -q to avoid a SIGPIPE masking a finding as clean, and the publisher refuses an unset/malformed/self-pointing mirror and plain-pushes (never force).
Could you re-review and lift the change-request if you agree?
4 participants
Summary
This repository is becoming the private development repo of the client; its public face will be a separate, deliverable-only mirror (charts, installer, README/LICENSE/docs, the Pages chart index, releases). This PR adds the fail-closed guard and publisher that is the only path from here to that mirror:
.publish-include— the explicit allowlist of what MAY ship (tracked files only;!lines take the chart test suites and CI values back out)..publish-include-pagesis the same for the gh-pages chart index..publish-forbidden—[paths]that must never appear in the staged tree even if allowlisted by mistake (test suites, CI, workflows, RFC and tooling docs, build files, Go source, credential-shaped files), and two tiers of string needles matched over every staged text file:[strings-refuse](mailboxes other than the allowlisted support one,arn:aws:, 12-digit ECR account ids) refuses on a hit;[strings-report](internal ticket/RFC identifiers, non-production tracebloc hostnames) is counted and printed — per-needle totals and the ten most-hit files — and refuses only under--strict.[allow]spares the one public mailbox. The parser refuses the list itself (exit 2) on an unknown section header, a needle listed in both tiers, or an empty[strings-refuse]— a guard with nothing to refuse is misconfigured. Customer and tenant identifiers are deliberately not committed — this file is public — they arrive at publish time from a secret (PUBLISH_FORBIDDEN_TENANTS), join the refuse tier, and the guard refuses to run the scan without them.scripts/publish-guard.sh— stages the allowlisted tree (and, when given, the release assets) into a clean directory and runs four guards over it: allowlist, forbidden paths, forbidden strings (both tiers;--strictpromotes the report tier),gitleaks detect --no-git --redact. Exit 0 only when all four ran and passed; 1 = a guard refused (it names the guard, the tier, the rule and the file:line — never the matched text); 2 = could not tell (unreadable/empty/malformed lists, zero tracked files, an allowlist matching nothing, a symlink, a missing or crashed scanner, a non-empty output dir). Every guard runs even after an earlier refusal, so one run reports everything.scripts/publish-mirror.sh—target(refuses an unset mirror, a malformed name, or a mirror equal to this repository),tree(clone / replace content / commit / plain push, never force),release(creates the tag at the pushed commit with every asset; refuses to overwrite an existing release)..github/workflows/mirror-publish.yaml— runs after Release Helm Chart completes (not onrelease: published, which fires before the charts, installer and signed manifest are attached and before gh-pages is pushed), plusworkflow_dispatchwithdry-run(default true) that runs every guard and prints the staged file list without pushing, andstrict(default false) that passes--strict; the repository variablePUBLISH_STRICT=truedoes the same for every run, including theworkflow_runpath that has no inputs. The mirror is named by theMIRROR_REPOvariable with no default: until the mirror exists the job refuses to publish. Publishing uses a GitHub App installation token scoped to the mirror only.What a dry run prints today
On the current tree (
--extra-forbiddenfed a placeholder tenant needle) the allowlist stages 91 of 369 tracked files,[forbidden-paths]is clean,[gitleaks]is clean, and[forbidden-strings]reports exit 0:arn:aws:, ECR account id, the placeholder tenant). Nothing in the deliverable trips the hard tier today.backend#668,RFC-0114,RFC-BACKEND10,tracebloc/backend9,rfcs#2; non-production hostsdev-api6,stg-api6,dev5,stg5 (*.tracebloc.io). Most-hit files:scripts/install-k8s.ps1121,client/values.yaml114,scripts/lib/install-client-helm.sh80,client/templates/_helpers.tpl69,client/values.schema.json63,client/templates/jobs-manager-deployment.yaml42,docs/SECURITY.md37,scripts/lib/cluster.sh26,scripts/lib/common.sh23,client/templates/secrets.yaml21. The hostnames are the chart'senv: dev|stg|prodswitch baking the non-production API hosts intovalues.yaml/_helpers.tplby design.The same run with
--strictexits 1 and refuses every one of those 825 lines under[strings-report (strict)]. A guard that could never pass was a guard nobody would arm; this one passes on the hard rules today and keeps the soft backlog measured until decision 3 below is taken. Nothing here gates a PR; the workflow only runs after a release or on dispatch.Related
Part of the public/private repository split.
Type of change
Test plan
bats scripts/tests/publish-guard.bats(47, was 39) andscripts/tests/publish-mirror.bats(17): every guard is mutation-proved against a fixture repository whose allowlist/forbidden lists are written in the test, and each refusal is asserted on the guard name, the tier and the offending path or needle. Tier cases: a report-tier hit alone → exit 0 with the per-needle count and the most-hit table; the same hit under--strict→ exit 1 naming[strings-report (strict)]; a refuse-tier hit → exit 1 naming[strings-refuse]; both in one run → refused and still counted; the table sums needles per file, largest first, ten rows at most. List cases: a needle in both tiers → exit 2 naming the duplicate; the retired[strings]header (and a header with a space) → exit 2 naming the unknown section for both scans that read the list; an empty[strings-refuse], absent or present-but-empty, → exit 2, judged before the private needles join. Fail-closed cases as before: empty/missing allowlist, allowlist matching nothing, empty/missing forbidden list, empty/missing private needle list, missing/crashing scanner, non-empty output dir, symlink, non-git source, empty assets. The committed lists get their own cases with inputs written in the test: every refuse-tier needle refused by name, every report-tier needle counted by name and refused under--strict(the summary's needle counts are asserted, so a needle added to the file without a planted input reddens the suite), every forbidden path class by name, and the real tree now asserted exit 0 — the refuse tier holds on the actual deliverable, and a refuse-tier needle landing in a deliverable file reddens CI. Each new check was mutation-proved by breaking it in the script and watching only its named tests redden.publish-mirror.batsdrivestreeagainst real bare repos overfile://(first publish, unchanged no-op, replace-with-appended-history, seeded branch not force-pushed, unreachable remote) andreleaseagainst a recordingghshim.make lint(parse + shellcheck error tier over the derived file set),shellcheck -S warning -xon both new scripts,bash scripts/check-style.sh,bats scripts/tests/bats-hygiene.bats,actionlinton the workflow, the orghouse-rules.sh --allandpipefail-early-close.sh(shell + yaml scopes): all clean.bash scripts/publish-guard.sh --source . --out /tmp/stageon the real tree: verdict above.Open decisions
MIRROR_REPOvariable. Until then every real run refuses at the target check.PUBLISH_FORBIDDEN_TENANTS— one extended regex per line. The guard refuses to run the string scan while the secret is empty.strictwhen decided. The report tier (internal ticket/RFC identifiers, non-production hostnames) is measured on every run, refused on none. The day the call is "strip", set the repository variablePUBLISH_STRICT=true(or dispatch withstrict: true) and the same needles refuse; the day it is "accept", delete the needles from[strings-report]. Nothing else in the guard changes either way.Deployment notes
MIRROR_REPO(bare repo name in this organisation) only once the mirror repository exists. Until then every real run refuses at the target check; dry runs work.PUBLISH_FORBIDDEN_TENANTS(one extended regex per line) before the first dry run is expected to pass — an empty list is refused as "could not tell".PUBLISH_STRICT(repository variable) is unset: the report tier is measured, not enforced. Setting it totruearms it for every run.RELEASE_TRAIN_APP_ID/RELEASE_TRAIN_APP_PRIVATE_KEYalready exist at the organisation level; the App must be installed on the mirror repository.Checklist
bash scripts/check-style.shpassesNote
High Risk
Introduces the sole publish path to a public mirror with write tokens, secret-driven tenant scanning, and branch/release replacement rules—misconfiguration or guard bypass would expose internal content or corrupt customer-facing installs.
Overview
Adds the only automated path from this private dev repo to a separate public, deliverable-only mirror: charts, installer, docs,
gh-pagesindex, and mirrored GitHub releases.Policy and guard: New
.publish-include/.publish-include-pagesallowlists and.publish-forbidden(path blocks plus refuse vs report string tiers; tenant needles come fromPUBLISH_FORBIDDEN_TENANTSat runtime).scripts/publish-guard.shstages tracked allowlisted content (and release assets), then runs four fail-closed checks: allowlist, forbidden paths, forbidden strings (+ optional--strict), and pinned gitleaks.Publisher:
scripts/publish-mirror.shresolves the mirror target (no default, refuses publishing back onto source), plain-pushes staged trees, and creates releases without overwriting existing tags.Workflow:
.github/workflows/mirror-publish.yamlruns after Release Helm Chart succeeds (not onrelease: published), defaults to dry-run, fetches release tags as data with SHA pinning, gates default-branch/gh-pagesupdates on newest stable only (prereleases and older stables get release-only), and uses a GitHub App token scoped to the mirror. Extensive bats coverage includes workflow shape/mutation tests.Reviewed by Cursor Bugbot for commit 37600f4. Bugbot is set up for automated code reviews on this repo. Configure here.