Repository navigation
Make the repo safe to point at publicly: docs hygiene + require explicit --orgs - #10
Conversation
This repo is about to be linked from a public dotCMS engineering blog post, so the sanitized incident report gets read by people outside the company. - the public copy was still labelled "Confidential — Incident Response", which reads as an accidental leak rather than a deliberate release - add a current-status pointer to the trust center; the document is an Aug 25 snapshot that says containment is in progress, and a reader arriving months later has no way to tell that it is stale - the usage example named a repo the report deliberately withholds, which narrows the redaction; a placeholder does the same job Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Applies the review suggestion on --orgs, and carries it through: the file header described the tool as sweeping dotCMS repos when it sweeps whichever org you point it at. Genericizing the examples exposed something worth stating outright rather than hiding: ORGS defaults to "dotCMS dotcms-community" (line 47). A stranger who clones this and runs it bare sweeps our orgs, not theirs. Left the default alone — changing it would alter behaviour for our own runs — but documented it in the usage block so it is a stated default rather than a surprise. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The public blog post sends strangers to this repo. A bare org_sweep.sh defaulted to ORGS="dotCMS dotcms-community", so the obvious first command someone runs after cloning pointed their token at our organisation rather than theirs. Removing the default alone would have been worse than leaving it: with no default and no guard the org loop iterates zero times, enumerates nothing and exits 0 -- a green sweep of no repositories, which is the exact failure class this suite exists to catch. So the default is gone AND one of --orgs/--repos/--local is now required, exiting 2 with a usage message. Test 10 asserts on the message rather than only the exit code: a missing token also exits 2, and a test that cannot tell those apart passes for the wrong reason. Suite: 24 pass, 0 fail. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Riding the Added:
Suite: 24 pass, 0 fail. |
|
Heads-up on a conflict that appeared after you opened this. When I trial-merged this onto #8 before pushing my review fixes, it was clean. It is not any more: we both append a case to the end of Trivial to resolve — keep both, yours becomes 13 — but whoever merges second inherits it, and the duplicate number survives a careless resolution. No interaction beyond that. The On the guard itself: it is the same defect as both of your blocking findings on #8 — a run that examines nothing and exits green. Three instances in three places, found independently, which is probably worth a line in the README as a class rather than three separate fixes. |
Resolves the test_org_sweep.sh conflict with #8: main's new cases 10-12 kept, the bare-run guard test renumbered to 13. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Sqwe4b1GxVAf2izuMACri9
fabrizzio-dotCMS
left a comment
There was a problem hiding this comment.
Requesting changes for one gap in the class this PR closes.
The guard checks that an argument was passed, not that anything was swept. --orgs with a typo prints SKIP could not enumerate org …, sweeps 0 repos and exits 0. Reproduced on this branch with --orgs zz-no-such-org-qq81; --orgs , does the same.
Suggest: a failed org enumeration, or 0 repos in --orgs mode, exits 2 — plus a test next to case 13.
Nit: the usage line org_sweep.sh --deep now hits the guard on its own; --orgs your-org --deep instead.
Review on #10: the bare-run guard proves an argument was passed, not that anything was swept. `--orgs` with a typo printed SKIP, enumerated nothing and exited 0; `--orgs ,` and `--repos ,` did the same. - zero repositories to sweep now aborts with exit 2 - an org that fails to enumerate drives exit 2 at the end of the run, after the orgs that did list are still swept; INCOMPLETE is printed alongside the summary - repo count is non-blank lines, so `--repos ,` no longer counts as one repo - usage: `--deep` alone hits the guard; the example now passes --orgs Case 14 runs offline (stub gh, insteadOf to a local bare repo) and fails 7 of 8 assertions against the previous org_sweep.sh. nightly_sweep.sh always passes a non-empty --repos batch, so it is unaffected. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Sqwe4b1GxVAf2izuMACri9
|
@fabrizzio-dotCMS good catch, thanks. Fixed in 2d47321:
Case 14 sits next to 13 and runs offline: a stub
|
fabrizzio-dotCMS
left a comment
There was a problem hiding this comment.
Verified 2d47321: suite 57/57 green; case 14 against the previous org_sweep.sh fails 7 of 8 as stated.
Non-blocking:
- A real org that is empty or only has archived repos now also aborts with
0 repositories to sweep … nothing was checked, which reads like a typo'd org name. note "sweeping … from: $ORGS"prints nothing afterfrom:under--repos; the newdieuses${ORGS:-$REPOS_ARG}.- The README line on the "examined nothing, exited green" class is still missing — worth adding here if you're touching it again.
This repo is about to be linked from a public dotCMS engineering blog post about the PolinRider campaign, so these docs get read by people outside the company — mostly security teams deciding whether to run the scanner.
Three docs fixes and one behaviour change:
incident-report-polinrider-dotcms.mdwas labelledClassification: Confidential — Incident Responsewhile being the deliberately-public sanitized copy. Not a leak, but "Confidential" on a doc we are actively linking reads as an accident to anyone who screenshots it. Now reads "Public copy — sanitized for release".The usage example in
org_sweep.shnamed a repo the report withholds. The report redacts the private repo (force-push to master, backdated ~13 months) as[private org repo — name withheld], but the sweep usage block used that same repo as its--reposexample. Together they narrow the redaction. A placeholder does the same job. If that repo is already on the advisory's published "what was affected" list this was moot either way — the placeholder is correct regardless.Stale status. The report is an Aug 25 snapshot stating containment is in progress and an implant live on master. It says it is a snapshot, but a reader arriving in late September from a blog post shouldn't land on that with no pointer. Added a "Current status" line directing to the trust center rather than asserting a resolution this PR can't verify.
A bare
org_sweep.shno longer sweeps our orgs. TheORGS="dotCMS dotcms-community"default is gone, and one of--orgs/--repos/--localis now required (exit 2 with a usage message). The guard matters: with no default and no guard the org loop runs zero times and exits 0, a green sweep of nothing. Test 13 pins it.nightly_sweep.sh(from fix the allowlist and the guard, and add the nightly driver the sweep needs to run from cron #8) always passes--repos, so it is unaffected.Audited while I was in here and found clean: no 40-character SHAs survived sanitization (the report warns one leaked in an earlier pass — it is gone), no employee or customer names anywhere, no Slack, Drive or helpdesk links.
Since #8 merged
Main was merged in rather than rebased. The only conflict was
test_org_sweep.sh: #8 added cases 10–12, and this PR's bare-run test is now case 13. The earlier warning about #8 (a batch that never ran reported green) was fixed before it merged, in 1eb53b8.🤖 Generated with Claude Code