Repository navigation
chore: prepare the repo for public visibility - #97
Merged
Merged
Conversation
Five things that either break or mislead once this repo is public: 1. The `staleness` docs gate called a reusable workflow out of a private repo with `secrets: inherit` — a public repo cannot do that, so the job would have failed on every PR touching docs/handbuch/ or src/. Vendored the checker as .github/scripts/docs-staleness.mjs, a port of the canonical SourcesHasher. Verified: it reproduces all four existing sources_hash values (including the `src/permissions/**` glob), fails when a documented source changes, and re-signs with --sign. 2. Issue intake ran on a self-hosted runner. Self-hosted runners on public repos are a standing risk and this job only shells out to `gh issue edit` — moved to ubuntu-latest. 3. No LICENSE and no `license` field: public without one means nobody may legally use it. MIT. 4. Instance hostnames: swapped the arbitrary fixture hosts in tests/, docs/ and examples/ for `mychurch`. Deliberately kept where the hostname is provenance rather than a placeholder — api-coverage.md's live-verification notes, permissions/catalog.json's `capturedFrom`, the dynamic fixtures' README — since those record which instance and CT version a finding came from. 5. Dangling references to private sibling repos, which 404 for every visitor: generalised to "your config repo" / "the consuming Handbuch site". Left the provenance mentions in src/ and tests/ comments alone; rewording those would force a docs re-sign for cosmetic reasons. Added CONTRIBUTING.md and SECURITY.md, and gave the README contributing and license sections. Prettier reports 102 pre-existing unformatted files on main (format:check is not in the CI gate). Left alone — that is a separate cleanup. Verified this branch adds no new ones. Claude-Session: https://claude.ai/code/session_012vnLUFPTuDVZ7DZhpm7sNt
Review of the vendored staleness checker found bugs in the direction that matters — the gate going green while the docs are stale. All three reproduced before fixing, and each repro re-run after: 1. A `#` comment inside a `sources:` list silently truncated the list. The comment did not match the list-item regex, so the parser left list mode and dropped every remaining entry. `--sign` then baked the truncated set in, and mutating a dropped source left the gate green forever. The parser is now strict: comments are skipped without ending the block, and any line it does not positively understand is an error rather than a skip. 2. `sources_exempt_reason: >` (a valid YAML block scalar) parsed to the literal ">", which is truthy, silently dropping the page from the gate entirely. Block scalars are now rejected with a message naming the key. 3. `--sign` on a quoted `sources_hash: "..."` reported success while writing nothing: the parser strips quotes, so the string-replace target never matched and String.replace returned the input unchanged. It now rewrites the whole line and errors if the content did not change. Also tightened `**` to a trailing prefix-walk only. Mid-pattern `src/**/*.ts` resolved to every file under src/, and a leading `**` walked the repo root, pulling .git/ and node_modules/ into the hash and making it depend on untracked local state. Both are now rejected rather than approximated. The four real pages still pass with their existing hashes unchanged. Other review findings: - ci.yml had no `permissions:` block, inheriting the org default (often read-write). Pinned to `contents: read`. - docs.yml did not watch .github/scripts/, so a PR weakening the checker did not run the gate. Added to both path filters. - `npm run lint` — which CONTRIBUTING.md and README.md tell contributors to run — failed with 1436 errors once docs/README.md's mkdocs build had been run, because site/ was not ignored. Added to eslint and prettier ignores. - tests/ctClient.test.ts:21 kept the instance slug in a `ChurchTools_ct_eqrm` cookie name that the hostname scrub missed. - Corrected prose that overstated the code: the keychain is macOS-only (no Linux/Windows backend), `assertNotPeople` guards writes but not reads (`ct get raw /persons` returns person records), live writes are gated by CT_LIVE + CT_LIVE_WRITE + a CT_LIVE_WRITE_HOST match rather than CT_LIVE alone, and SECURITY.md now owns the `security -w` argv/ps exposure that tokenStore.ts already documents. Claude-Session: https://claude.ai/code/session_012vnLUFPTuDVZ7DZhpm7sNt
The two comments citing `modifiedPid 1/3891` named the ChurchTools person ids of the admins who authored those grants on prod. Unresolvable from outside the instance, so not PII in practice, but nothing in the explanation depends on the numbers — the point is that the rows carry a real admin's pid rather than the system baseline's -1. Comment-only, so the page content is unaffected; permissions.md is re-signed because `src/permissions/**` is one of its declared sources. The gate flagged this on its own, which is the behaviour it exists for. Claude-Session: https://claude.ai/code/session_012vnLUFPTuDVZ7DZhpm7sNt
Follow-up review findings. None was a silent pass — the gate erred loudly in
the safe direction in every case — but all three would have surprised someone.
The strict parser rejected any block list or nested mapping under a key it does
not read, so standard mkdocs-material frontmatter failed:
hide:
- navigation → cannot parse frontmatter line: - navigation
That matters here because docs/handbuch/ is built with mkdocs-material, where
`hide:` is the normal per-page directive. Indented continuations under keys
other than `sources:` are now skipped — they are not this gate's business —
while `sources:` keeps its any-line-not-understood-is-an-error rule, which is
where the silent-truncation risk actually lives. Verified the two do not
interact: a page with `hide:`/`extra:` blocks still captures both `sources:`
entries (hash matches the computed ground truth for that file set), and
mutating a declared source still goes stale. Key names may now be capitalised
or hyphenated, and the parse error names the shape it expects.
CRLF: `startsWith("---\n")` failed on `---\r\n`, so a Windows checkout with the
default autocrlf reported "no frontmatter" on all five pages. The parser now
normalises line endings, and .gitattributes pins text to LF so the situation
does not arise — the real fix, since the gate hashes file bytes and a CRLF
checkout would otherwise compute different sources_hash values than CI.
`---\n---\n` reported "unterminated frontmatter" because the search started
past its own closing delimiter; it now reports the accurate "no `sources:`".
Also reworded SECURITY.md's people guarantee: the load-bearing control is the
resource registry (there is no person resource kind), with assertNotPeople as a
second check on top. The reviewer noted the previous wording leaned on the
guard by name when the registry is what actually holds.
Claude-Session: https://claude.ai/code/session_012vnLUFPTuDVZ7DZhpm7sNt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Everything that either breaks or misleads once this repo is public. Prompted by wanting to flip it to public as a showcase.
What was actually blocking
The docs staleness gate would have failed on every PR. It called a reusable workflow out of a private repo with
secrets: inherit— a public repo cannot do that. Rather than drop the gate, it is vendored as.github/scripts/docs-staleness.mjs, a port of the canonicalSourcesHasher. Verified to reproduce all four existingsources_hashvalues, including thesrc/permissions/**glob.Issue intake ran on a self-hosted runner. Self-hosted runners on public repos are a standing risk; the job only shells out to
gh issue edit, so it moved toubuntu-latest.No LICENSE and no
licensefield — public without one means nobody may legally use it. MIT.ci.ymlhad nopermissions:block, inheriting the org default (often read-write). Pinned tocontents: read.Review findings, folded in
A review of the vendored gate found three ways it could go green while the docs were stale — the exact failure it exists to prevent. All three reproduced before fixing, each repro re-run after:
#comment inside asources:list silently truncated it.--signthen baked the truncation in, and mutating a dropped source left the gate green permanently.sources_exempt_reason: >(valid YAML) parsed to the literal">", truthy — silently dropping the page from the gate.--signon a quotedsources_hashreported success while writing nothing, leaving the page stuck stale.The parser is now strict: comments never end a block, and any line it does not positively understand is an error rather than a skip.
**is restricted to a trailing prefix-walk — mid-patternsrc/**/*.tshad resolved to every file undersrc/, and a leading**walked the repo root, pulling.git/andnode_modules/into the hash.Also corrected four prose claims that overstated the code: the keychain is macOS-only,
assertNotPeopleguards writes but not reads (ct get raw /personsreturns person records), live writes needCT_LIVE+CT_LIVE_WRITE+ aCT_LIVE_WRITE_HOSTmatch rather thanCT_LIVEalone, and SECURITY.md now owns thesecurity -wargv exposure thattokenStore.tsalready documented.Instance data
Fixture hostnames genericised to
mychurch; kept where the hostname is provenance rather than a placeholder (capturedFrom, the live-verification notes inapi-coverage.md, the dynamic fixtures' README) — those record which instance and CT version a finding came from. Note this is genericisation, not concealment: the hostname appears in 24 commits of history, which is an accepted call.Swept for PII across every tracked file and every history blob — no person-data fields, no contact details, no person names; fixture values are group/role ids and German label strings only. Two comments citing admin
modifiedPids were genericised.Also
docs.ymlnow watches.github/scripts/, so a PR weakening the checker actually runs the gate.site/added to eslint/prettier ignores:npm run lint, which CONTRIBUTING and README tell contributors to run, failed with 1436 errors once the mkdocs build had been run locally.Not in scope
Prettier reports 102 pre-existing unformatted files on
main(format:checkis not in the CI gate). Left alone as a separate cleanup; this branch adds none.Verification
npm run lint,npm run typecheck,npm test(570 passed / 5 skipped),npm run build,mkdocs build --strict, and the staleness gate all pass.https://claude.ai/code/session_012vnLUFPTuDVZ7DZhpm7sNt