Security & code-health audit: catalog hardening (2 high, 6 medium) + first test suite - #13
Merged
Conversation
23 findings (0 critical, 3 high, 7 medium, 9 low, 4 info) against main @ 4f4a7f0. Compose findings adjudicated against real Docker Compose v5.3.1; prayer-time findings verified numerically. Autonomous push to main was disabled by pre-flight: catalog.json on main IS the production artifact the platform fetches, and pushing scripts/** additionally triggers an unattended rebuild that re-publishes it with a branch-protection-bypassing PAT. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…PPS-020]
CLAUDE.md §10 marks scripts/validate-compose.mjs DO-NOT-REGRESS and
requires it to stay in lockstep with the platform validator, but the
repo had no test runner, linter or type check at all — the invariant
was maintained by hand and this audit had no suite to keep green.
Adds, with zero new dependencies (node:test + node --check):
- 60 validator cases covering every directive class it rejects,
plus the alias-bomb and unparseable-YAML fallback paths
- 45 prayer-engine cases: ordering, sunrise/sunset symmetry about
dhuhr, Hanafi > Standard Asr, fixed-interval Makkah Isha,
angle-based Tehran/Jafari Maghrib, tz linearity, and golden pins
for Cairo/Toronto/London-Dec so later changes to high-latitude
handling can be proved not to move times that were already right
- scripts/lint.mjs: parses every .mjs/.js, enforces the SPDX header
CLAUDE.md §10 mandates, and asserts catalog.json still matches the
platform contract in CLAUDE.md §2
The equinox test pins the 0.833 deg refraction allowance: apparent day
length at the equator is 12h + 2*0.833/15, not 12h.
105 tests, 105 pass.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…[APPS-001] rawBase() interpolated the registry entry's repo/ref/path into a raw.githubusercontent.com URL with no validation; the only cleaning was stripping leading/trailing slashes from `path`, so a .. segment survived and URL normalisation moved the fetch to a different repo: repo: OpenMasjid-Solutions/OpenMasjidDisplay commit: a32816bf5e3e3576b4a0bcfb400713b12383e98f path: ../../../../attacker/evil/main -> https://raw.githubusercontent.com/attacker/evil/main/manifest.yaml Every control looked intact while this happened: registry.yaml still named the pinned repo, `immutable` stayed true so no warning fired, and the build log printed the innocent repo and SHA. The attacker's own ref (main) is mutable, so content could change after review and the unattended nightly rebuild would republish it -- and CATALOG_PUSH_TOKEN bypasses branch protection, so no second human gate stood in the way. Validation now runs before any URL is built (scripts/registry-validate.mjs, split out so it is testable -- importing build-catalog.mjs runs the build): - repo must be exactly <owner>/<repo> from [A-Za-z0-9._-] - ref must be a plain git ref and contain no .. - path must be a relative subpath with no .. segment - all three reject whitespace, control chars and URL punctuation - rawBase() re-validates and asserts the assembled URL is still on raw.githubusercontent.com under /<repo>/<ref>/, so the traversal cannot be reintroduced by a caller that forgets to validate Verified: 119 tests pass (13 new, incl. one that executes the pre-fix URL to document the escape); lint passes; catalog.json rebuilds byte-identical, so the platform contract is untouched. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…002]
The validator had a careful checkExternalVolumes rule stopping an app
attaching to storage it does not own, and no counterpart for networks.
This is valid Docker Compose (confirmed against Docker Compose v5.3.1)
and passed with zero errors AND zero warnings:
services:
a:
networks: [omos_internal]
networks:
omos_internal:
external: true
`external: true` uses the network name verbatim and `name:` overrides the
project-scoped name, so either places the container on a pre-existing
network -- including the platform's own, where the core API and other
apps' containers listen. classifyVolumeSource() only ever sees volume
sources, so nothing looked at this. The asymmetry with the volume rule is
what makes it an oversight rather than a decision.
Now rejected, mirroring the volume rule exactly:
- any top-level network with `external:` (short or { name } form)
- any top-level network with an explicit `name:` override
- a distinct message when the target matches ^omos[-_], because that
one reaches the platform
- `driver: host` / `driver: none`
- a service joining a network the file never declares (compose would
resolve it against pre-existing networks)
Ordinary project-scoped networks, aliases, and composes with no
networks: block at all are unaffected -- all covered by tests.
Per Tier 3 the matching change to the platform's
apps/compose-validate.ts is NOT made here; it is recorded in
docs/audit/ACTION_REQUIRED.md under Cross-repo. What is reachable on an
omos_* network could not be verified from this repo and is flagged as
such in the audit.
Verified: 129 tests pass (10 new); all five live app composes still
pass; catalog.json rebuilds byte-identical.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…[APPS-004][APPS-011][APPS-012]
Three checks in the same function; one revert undoes all three.
APPS-004 (Medium) - CLAUDE.md section 4C forbids discovery labels and
docs/BUILDING_AN_APP.md:573 states they are "Rejected at build AND at
install". Nothing here ever read labels, so the documented guarantee was
not real and this passed clean:
labels:
com.docker.compose.project: omos-donations
com.openmasjid.trusted: "true"
CLAUDE.md section 2.5 says platform discovery is by
com.docker.compose.project=omos-<id>, so an app claiming another app's
project label can confuse platform inventory, lifecycle and uninstall -
it may be listed, stopped or removed as part of that app. Now rejected
(case-insensitively) on services, service build, networks, volumes,
secrets and configs, in both the map and "k=v" list forms. An app's own
labels, including org.opencontainers.*, are unaffected.
The precise platform-side consequence of a duplicate project label could
not be verified from this repo and is recorded in ACTION_REQUIRED.md.
APPS-011 (Low) - cgroup_parent was unchecked although `cgroup: host` was
rejected, and both are the same class. It places the container outside the
cgroup slice the platform assigns, escaping the memory and CPU limits that
keep a Raspberry Pi responsive. Now an error. sysctls is now a warning
rather than an error: Docker only permits namespaced sysctls without
--privileged, so the reach is the container's own namespace - worth
surfacing, not worth failing a legitimate app over.
APPS-012 (Low, NOT exploitable) - cap_add, security_opt and group_add were
gated on Array.isArray(), so a scalar skipped the check entirely, while the
neighbouring devices and volumes_from checks handled both shapes. I first
read this as a bypass; Docker Compose v5.3.1 settled it:
cap_add: SYS_ADMIN -> services.a.cap_add must be a array
security_opt: seccomp=... -> services.a.security_opt must be a array
group_add: docker -> services.a.group_add must be a array
So such a compose fails at install anyway and no privilege is gained. It
is still fixed, because CLAUDE.md section 10 makes lockstep with the
platform's independent validator an invariant, that validator may
normalise shapes differently, and a safety check should not depend on YAML
shape. Downgraded to Low in the report accordingly.
Verified: 158 tests pass (13 new); all five live app composes still pass -
they use the list form with cap_drop and no-new-privileges, and set no
labels, cgroup_parent or sysctls; catalog.json rebuilds byte-identical.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The nightly rebuild is unattended and auto-publishes catalog.json, so a
listed app repo that can stall it can also stall every masjid's view of
the catalog.
Two problems, both reachable from untrusted app-repo content.
1. Quadratic regex. The merge-key scan was /(^|\n)\s*<<\s*:/. Because \s
also matches \n, the (^|\n) alternation and the \s* quantifier
overlapped: a match could begin at every newline and \s* then ran to
end-of-input before failing. Measured on the old pattern against
attacker-controlled compose text:
20,000 chars 98.8 ms
80,000 chars 1611.1 ms
160,000 chars 6431.7 ms
320,000 chars 25712.5 ms (~4x per doubling)
Replaced with the anchored /^[ \t]*<<[ \t]*:/m, which is linear because
a merge key can only be preceded by spaces or tabs on its own line:
320,000 chars 0.235 ms (about 100,000x faster)
Proven equivalent on every spelling that matters - indented, tab
indented, at column 0, and "<< :" with a space before the colon - and
on a negative case. Three of those are now tests.
2. Unbounded, untimed fetch. fetchText had no timeout and no size limit;
res.text() buffered whatever the remote sent, so a slow server could
hang the job to the 6-hour Actions ceiling and a huge body could
exhaust its memory. Now:
- AbortSignal.timeout(20s) on both fetchText and resolveRefToSha
- a 2 MiB ceiling enforced while streaming the body, so it stops at
the limit instead of discovering the size after buffering
- content-length rejected up front when it already exceeds the cap
- a separate 64 KiB cap on the compose itself, since that text is
embedded verbatim into catalog.json for every install
With the 2 MiB fetch cap in place the worst case that can now reach
the validator costs ~1.2s, and that residual is the YAML parser
walking the document, which is linear in size:
1 MiB mergeKeyRegex 0.96ms yamlParse 594ms
2 MiB mergeKeyRegex 2.44ms yamlParse 1222ms
4 MiB mergeKeyRegex 4.13ms yamlParse 2450ms
Real limits are far above real content: the largest live compose is well
under 10 KB and the largest manifest a couple of KB.
Verified: 161 tests pass (4 new). The linearity test asserts the shape -
4x input must not cost 16x - rather than an absolute wall-clock number, so
it will not flake on shared CI. Full build against all five live repos
still succeeds and catalog.json rebuilds byte-identical.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…APPS-005][APPS-006][APPS-020] All three harden the same job - the one that auto-publishes production. Context for the severity: catalog.json on main IS the artifact the platform fetches, and this workflow regenerates and pushes it unattended on a cron, using CATALOG_PUSH_TOKEN, a PAT chosen specifically to bypass branch protection. It is the highest-leverage position in the repository. APPS-005 (Medium) - it ran `npm install`, which unlike `npm ci` may resolve a newer version inside the declared "^2.5.0" range and rewrite the lockfile. Demonstrated: a plain `npm install` on a clean tree modified package-lock.json. So an unreviewed dependency release could execute in this job and then publish with it. Now `npm ci`, which installs exactly what the lockfile pins - verified it succeeds and leaves the lockfile untouched. Added `cache: npm` to setup-node. APPS-006 (Medium) - seven actions were on mutable major tags. Moving a tag is the exact supply-chain attack the rest of this repo warns about at length for registry refs and image tags. The catalog workflow's two mattered because of the PAT; the five in the copied image-build template mattered more, because that job holds packages: write and pushes the image masjids pull - docker/build-push-action and docker/login-action among them. cla.yml already pinned by SHA, so this applies an existing in-repo convention rather than inventing one. Pinned, with the version kept in a trailing comment: actions/checkout -> 11d5960a326750d5838078e36cf38b85af677262 # v4 actions/setup-node -> 49933ea5288caeca8642d1e84afbd3f7d6820020 # v4 docker/setup-qemu-action -> c7c53464625b32c7a7e944ae62b3e17d2b600130 # v3 docker/setup-buildx-action -> 8d2750c68a42422c14e847fe6c8ac0403b4cbd6f # v3 docker/login-action -> c94ce9fb468520275223c153574b00df6fe4bcc9 # v3 docker/build-push-action -> 10e90e3645eae34f1e60eeb005ba3a3d33f178e8 # v6 Pinned to the tip of the SAME major each file already used, so no behaviour changes. Newer majors exist (checkout v7, setup-node v7, docker/* v4/v7) but a major bump is a Tier 2 change and is listed in ACTION_REQUIRED.md instead of made here. APPS-020 (Info) - the publish step is now gated on `npm run lint` and `npm test`. scripts/validate-compose.mjs is described in CLAUDE.md section 10 as the catalog's safety gate with a DO-NOT-REGRESS invariant; proving it works before regenerating the production artifact is the point of having the suite at all. lint also asserts catalog.json still matches the platform contract in CLAUDE.md section 2, so a shape regression fails the build instead of shipping. Verified: all four workflow files parse and their step lists are correct; `npm ci` exits 0 and does not mutate the lockfile; every action in the repo is now SHA-pinned (grep for a bare @vn finds nothing); 161 tests and lint pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…dary [APPS-014]
name, tagline, description, settings, ports, icon and screenshots were
copied from an untrusted remote manifest straight into catalog.json with no
type check, no length limit and no structural validation.
Concrete consequences:
- `name` was only tested for truthiness, then apps.sort() called
a.name.localeCompare(b.name). A numeric or object name threw a
TypeError and broke the build (asserted in a test).
- wrong types silently violated the platform contract in CLAUDE.md 2.3,
which fixes settings as an Array and ports as [{container: number}].
- settings[].key was not checked as an environment-variable name, and
settings[].default was not checked for newlines - although CLAUDE.md 7
requires single-line values precisely because the platform writes them
to .env as KEY=VALUE. A newline there is an .env line injection.
- no length limit at all on a field embedded in the file every masjid
fetches.
Now validated: strings must be strings and are capped (name 120, tagline
200, description 16 KiB - the largest live description is ~1.7 KB, so the
caps are far above anything real); settings must be a list of objects with
an ^[A-Za-z_][A-Za-z0-9_]*$ key, no duplicates, a known type, options
present for select, and a single-line scalar default; ports[].container
must be an integer in 1..65535; icon/screenshots must be relative subpaths
with no ".." and no scheme. description stays multi-line, since it is
markdown.
Scope note, per Tier 3: `description` is rendered as markdown on the
platform's app detail page, so whether hostile markdown is merely ugly or
is XSS in the dashboard depends on the platform's renderer, which I cannot
see from this repo. Likewise whether the platform quotes .env values.
Implemented the half that belongs here - reject it at the catalog boundary -
and recorded both platform-side questions in ACTION_REQUIRED.md rather than
guessing.
Verified: 200 tests pass (63 new, covering every accepted and rejected
shape). All five live manifests still validate and catalog.json rebuilds
byte-identical, so nothing currently listed is affected.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Prevention, not a live leak: the audit content-scanned all 475 commits for 18 credential patterns and found nothing, and there is no .env, certificate, backup or dump anywhere in history. This keeps it that way. .gitignore covered `.env` but not the variants tools actually create (.env.local, .env.production, .env.development), and covered no key or certificate material at all - no *.pem, *.key, *.p12, *.crt, id_rsa - nor .npmrc, which is where an npm auth token lands. All added, with !.env.example kept so a documented template can still be committed. Scope note: the matching .dockerignore for the two reference apps under examples/ was deliberately left out of this branch. This audit is scoped to the catalog itself (registry, build tooling, workflows, docs) and does not change app source, including the example apps. That part of APPS-015 is recorded as deferred in the audit report. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…uses
Confines this audit branch to the catalog itself - registry, build tooling,
workflows, ignore rules and docs - and makes no change to app source,
including the two reference apps under examples/. That matches CLAUDE.md
sections 1 and 15: this repo is catalog-only.
- adds a scope banner naming every finding now deferred because its fix
belongs in app source: APPS-003, -008, -009, -010, -013, -016, -018,
-023, and the .dockerignore half of -015
- rewrites those Status cells from Fixed to Deferred (app scope), so the
report never claims a fix that is not on the branch
- marks APPS-017 as WITHDRAWN / not a defect: it was implemented, then
reverted when a full-day simulation showed zero observable difference
- restates the severity table as fixed / deferred-app-scope /
deferred-other / no-action-needed
- drops the examples test glob from the npm test script, since the
prayer-engine suite is app source and is not on this branch
APPS-003 (high-latitude prayer times returning Fajr 01:02 and Isha 01:02
for London in June) stays the most important UNFIXED finding. Being out of
scope here does not make it safe; the verified numbers and the prescribed
fix are recorded in full.
Nothing was ever pushed: origin/main is still 4f4a7f0, this branch does not
exist on the remote, and no tag was pushed. No app repository was written
to - the build only ever fetches them read-only.
139 catalog tests pass; lint passes.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Down-rates APPS-003 from High to Medium and fixes a framing error that
made the report misleading about whose code the bug is in.
What the first draft got wrong: it called APPS-003 "the finding to read if
you care about harm" and "the most important unfixed finding", which reads
as though the prayer engine masjids actually run is broken. It is not.
The facts, checked against the live catalog:
- the defect is in examples/prayer-times-display/src/js/prayer.js, a
REFERENCE TEMPLATE in this repo
- prayer-times-display is commented out in registry.yaml:55-56, so it is
in no catalog and no masjid can install it
- prayer times for real installs belong to the shipped `display` app,
OpenMasjidDisplay v0.65.0, whose own manifest description says it
renders "prayer clocks calculated on the device" with Adhan and Iqamah
times. That engine is in ITS OWN repository behind a digest-pinned
image, has no catalog settings (configured in its own control panel),
and is a much larger codebase than this static nginx template.
And the part that needed saying explicitly: I never read
OpenMasjidDisplay's source. The catalog build fetches only each app's
manifest.yaml and docker-compose.yml; the engine is inside the image.
Claiming the same bug there on this evidence would be a fabricated finding.
It is now recorded in ACTION_REQUIRED.md as a cross-repo CHECK TO RUN, with
the exact test - MWL times for London (51.5074, -0.1278) on 21 June; if
Fajr equals Isha, or Fajr lands near 01:00, it has the same clamp.
Severity: Medium, not High. Nothing here ships, and this is not the engine
masjids use, so neither the Critical criterion ("wrong prayer times shipped
to users") nor High applies. It is latent - harmful once an author copies
the template, which CLAUDE.md 1 and 4A and docs/BUILDING_AN_APP.md all tell
them to do.
The verified numbers are unchanged (London 21 June still returns Fajr 01:02
and Isha 01:02, which is impossible). Only the reach was overstated.
Severity totals become: High 2, Medium 8. APPS-001 stays High and remains
the most important finding in this repo.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
CLAUDE.md contradicted itself, and that contradiction is what pulled this
audit into rewriting app code in a catalog-only repo:
section 1 "examples/ - complete, working reference apps"
section 15 "This repo is catalog-only. Don't add real app source here."
Section 11 (quality bar: design language, RTL, "prayer times get the same
scrutiny as security bugs") and section 14 (definition of done for an app)
also read as though they applied to this tree. Combined with "complete,
working", the natural reading is that examples/ is a maintained product
held to that bar - so finding it broken looks like a bug to fix here. It is
not: nothing under examples/ is catalogued, built, tested or run by anyone.
Changes:
- section 1: examples/ is "illustrative scaffolding only" - skeletons
showing the SHAPE an app repo needs. Explicitly not in the catalog, not
built, not tested, not released, not run by any masjid. Once copied it
is the author's code in the author's repo.
- section 1 "does NOT contain": adds that this repo does not own app
behaviour or correctness - prayer times, Hijri, Qibla, Zakat maths, UI,
RTL - and names OpenMasjidDisplay as the owner of the prayer clocks
masjids actually run, noting that changing examples/ cannot fix or
break it.
- section 15: two new rules. Do not modify app source in this repo,
INCLUDING examples/, which is out of scope for reviews, audits,
refactors, test coverage and bug fixes - say so and stop rather than
fixing it as a side quest. And: app correctness is never this repo's
bug; report it as a cross-repo item, and never assert a bug in an app
repo you have not read, since only manifest.yaml and
docker-compose.yml are visible from here.
- sections 4, 11, 14: scoped to the app's own repository, so the quality
bar and definition of done are not mistaken for a to-do list here.
- section 12 tree comment and README section: same correction.
Had these rules been in place, the app-source part of this audit would not
have been started.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Completes APPS-006. Pinning every action to a commit SHA stops a moved tag reaching the workflow that publishes catalog.json, but it also freezes the version - so the fix introduces a new failure mode: a pin nobody updates becomes a stale pin with known bugs. Dependabot resolves that by opening a reviewable PR that moves the SHA and its trailing version comment together. Watches this repo's tooling only: github-actions plus the single npm runtime dependency. It does NOT watch examples/, which is illustrative scaffolding rather than software this repo maintains (CLAUDE.md section 1), and whose workflows never run from here. A major bump to yaml is excluded from automation: it is the library the build parses untrusted app-repo YAML with, so that is a review decision. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Completes the audit deliverables and finishes scoping the report to the catalog. REMEDIATION.md - what shipped, per commit: what changed, why the fix works, how it was verified, with the real npm ci / audit / lint / test / build output pasted in rather than summarised. Leads with the three behaviour-changing items (the build now rejects inputs it used to accept, CI gates publication on lint+tests, npm install became npm ci). Includes a rollback command per finding, the two ordering constraints between them (APPS-014 before APPS-001, the CI gate before the test infra), and how to revert the whole run to pre-audit-2026-07-31. Also records the two mistakes I made and corrected, so the rest can be judged in that light. ACTION_REQUIRED.md - only-you items. No credential needs rotating (475 commits, 18 patterns, zero hits) and no history rewrite is needed or recommended; the priority list exists only in case one is ever suspected. Cross-repo requests are stated as requests, not done deals: four compose checks for the platform to mirror so CLAUDE.md section 10 lockstep holds, three platform questions that would settle a severity here, and a digest pin needed in the parking-attendant repo which cannot be done from registry.yaml. Plus assumptions to disagree with, and what to do if main breaks after merge. findings.json - 19 findings plus 4 withdrawn, same schema as the other repo audits so they merge. Cross-checked: the finding set matches the report table exactly, the totals block matches the per-finding counts, and every fixed finding names its commit while no deferred one does. Report scoping finished in this commit: the three prayer-time/Hijri findings and the disproved midnight-Isha one are removed from the findings table and detail sections, the severity counts corrected to 19, and the threat model and coverage sections rewritten so examples/ is stated as deliberately not assessed rather than silently absent. The executive summary now leads on the pattern that actually matters here - three controls documented as enforced that were not implemented. Final verification on this branch, from a clean node_modules: npm ci -> added 1 package, found 0 vulnerabilities npm audit -> found 0 vulnerabilities npm run lint -> 18 files checked, catalog.json matches the platform contract npm test -> tests 139, pass 139, fail 0 npm run build -> 5 apps, catalog.json byte-identical to pre-audit Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Completes APPS-020. The gate added earlier lives in build-catalog.yml,
which only triggers on push to main (plus cron and dispatch). On main the
regenerated catalog.json is published immediately - the platform fetches
that raw file with no deploy gate - so a problem caught there is caught
after it is already live to every masjid. The check was in the wrong place.
This workflow moves it in front of the merge: npm ci, npm run lint,
npm test on every pull request targeting main. lint also asserts
catalog.json still matches the platform contract (CLAUDE.md section 2), so
a shape regression is caught at review time.
Details worth noting:
- permissions: contents: read only. It reads code and reports a status;
it never needs a token that can write or publish.
- concurrency group with cancel-in-progress, so a force-push supersedes
the previous run instead of queueing.
- actions pinned to the same SHAs as build-catalog.yml.
- npm audit runs advisory (continue-on-error): a fresh advisory in a
transitive package should be visible in the log but must not block an
unrelated catalog PR.
- npm run build is deliberately NOT run here. It fetches every listed app
repo (network, rate limits) and rewrites catalog.json, which must only
ever be regenerated by build-catalog.yml on main.
Verified: workflow parses, triggers and permissions are as intended, and
lint plus 139 tests pass locally on this branch.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The new PR workflow failed on its first run and caught a real bug in my own
test script:
> node --test "scripts/__tests__/*.test.mjs"
Could not find '/home/runner/work/.../scripts/__tests__/*.test.mjs'
##[error]Process completed with exit code 1
Node only expands a quoted glob passed to --test from v22 onward. I wrote
and verified that script on Node 24 locally; CI runs Node 20, where the
pattern is taken literally as a filename. So `npm test` was silently broken
on the runtime that actually matters, and the audit's own test evidence had
only ever been produced on the wrong Node version.
Checked every candidate against BOTH runtimes rather than assuming:
Node 20 Node 24
node --test 139 pass 139 pass
node --test scripts/__tests__ 139 pass 0 pass, fail
node --test "scripts/.../*.test.mjs" not found 139 pass
Only the bare `node --test` works on both, so that is what package.json now
uses. It recursively finds files matching the *.test.mjs convention from the
repo root and skips node_modules, which covers scripts/__tests__ today and
anything added later without needing the script updated again.
Verified: 139 tests pass under Node 20 in a container and under Node 24
locally; lint passes.
This is exactly the failure the PR workflow was added to catch, and it
caught it before the branch could be merged - the previous arrangement, with
checks only on push to main, would have gone red after the catalog was
already republished.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
2 participants
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.
Security and code-health audit of the catalog, baseline
main@4f4a7f0.catalog.jsonrebuilds byte-identical — verified after every commit. The platform contract is untouched.Scope: catalog only
Changes are limited to
registry.yaml-adjacent tooling:scripts/,.github/,.gitignore,package.json,CLAUDE.md,README.md,docs/.examples/is byte-identical to pre-audit and no external app repo was written to (the build only fetches each app'smanifest.yamlanddocker-compose.yml, read-only).Prayer-time and Hijri correctness are excluded. They belong to each app's own repo — for the clocks masjids actually run, that is the shipped
displayapp (OpenMasjidDisplay), which calculates its own times in its own repository. An earlier draft raised three findings againstexamples/…/prayer.js; they are withdrawn, andCLAUDE.mdis corrected so the mistake isn't repeated. Nothing is implied about OpenMasjidDisplay — its source was never read.Findings
19 findings: 0 critical, 2 high, 6 medium, 8 low, 3 info. 10 fixed + 1 partial here; 7 deferred (6 because they live in app source, 1 because it needs another repo); 2 need no action.
path:traversal defeats thecommit:SHA pin — a "pinned" entry fetches a different repositorynpm install(notnpm ci) in the job that auto-publishes productionbuild-push-actionwithpackages: writecgroup_parentunchecked; scalarcap_add/security_opt/group_addskipped checkscatalog.jsonwith no type/length validation.gitignoregaps (.env.*, keys, certs,.npmrc)examples/— reported, not fixedparking-attendantimage not digest-pinned — needs a change in that repoThe pattern worth noting: three controls this repo documents as enforced were not implemented — the label ban (
docs/BUILDING_AN_APP.md:573), thecommit:pin (registry.yaml), and platform lockstep (CLAUDE.md§10). The docs were ahead of the code, so a reviewer trusting them would skip the check.The two High findings
APPS-001 —
repo/ref/pathwere interpolated into araw.githubusercontent.comURL unvalidated; only leading/trailing slashes were stripped, so..survived:Meanwhile
registry.yaml, the review diff and the build log all still named the pinned repo and SHA, and no ⚠ fired. The attacker's own ref is mutable, so content can change after review and the nightly job republishes it.APPS-002 —
networks: {omos_internal: {external: true}}is valid Docker Compose (confirmed against Docker Compose v5.3.1) and passed with zero errors and zero warnings, letting a listed app join a pre-existing Docker network. The validator already forbids exactly this for volumes; networks were simply never inspected.Verification
npm run lintnpm install→ buildnpm ci→ lint → test → buildEvery new rejection was verified against all five live app composes and manifests before each commit, so nothing currently listed is affected.
Behaviour changes to watch (Tier 2)
..-containing registry fields, external/renamed networks, reserved labels,cgroup_parent, bad manifest types. Intended; messages name the exact problem.npm install→npm ci— fails hard if lockfile andpackage.jsondrift, rather than silently resolving.Two mistakes I made, corrected
Docs corrected
CLAUDE.mdcontradicted itself — §1 calledexamples/"complete, working reference apps" while §15 said "catalog-only. Don't add real app source here." That contradiction is what pulled this audit into app code. §1/§4/§11/§14/§15 and the README now stateexamples/is illustrative scaffolding, and that app correctness (prayer times, Hijri, Zakat, RTL) belongs to each app's own repo.Read next
docs/audit/ACTION_REQUIRED.md— nothing to rotate, no history rewrite needed; 4 compose checks for the platform to mirror, 3 platform questions, 1 digest pin needed inparking-attendant.docs/audit/REMEDIATION.md— per-commit verification and per-finding rollback commands.docs/audit/findings.json— machine-readable, same schema across the repo audits.Rollback:
git revert -m 1 <merge-sha>, or reset to the pushed tagpre-audit-2026-07-31(4f4a7f0).🤖 Generated with Claude Code