Release Please Beta on v3.76.1-beta.0 hard-failed at the very first
`gh release view "$TAG"` call:
failed to run git: fatal: not a git repository
(or any of the parent directories): .git
The reusable `whatsnew-highlights.yml` (PR #703) doesn't run
actions/checkout — so when `gh` tried to infer the target repo from
the runner's empty workspace it errored out. The first time it ran
against an actual release (#709 → 3.76.1-beta.0), the whole job died
before the deterministic-fallback path could save it.
Two changes, both single-line:
1. `env.GH_REPO: ${{ github.repository }}` at job scope. `gh` honours
this and won't fall back to parsing `.git/config`, so no checkout
is needed (the workflow only calls the GitHub API, never reads
repo files).
2. `continue-on-error: true` on the "Extract Features" step. The
file's comments say "never let highlights break a release", but
the original wiring only soft-failed the AI + inject steps. A
transient API hiccup at extract still hard-failed the whole job —
defeating the design intent. Match the comment.
Why not just add actions/checkout? It would work, but pulls the whole
repo over the wire on every release just for `gh` to read its own
config. GH_REPO is the lighter idiom.
Net impact today: v3.76.1-beta.0 shipped without the `<!-- whatsnew -->`
block; the app's parseWhatsNew() already falls back to the raw Features
list so the admin "What's New" banner still works. The next beta release
will pick up the polished version.
If GitHub Models is disabled for the org the ai-inference step errors;
without continue-on-error the job would go red and skip the inject+fallback.
Mark it continue-on-error so an unavailable Models cleanly degrades to the
deterministic bullets — the feature now works with Models off, not just on.
Activate the What's New highlights step that condenses each release's
Features into <=8 short bullets and injects a <!-- whatsnew --> block the
app reads (utils/whatsNew.parseWhatsNew), with a deterministic fallback.
Runs as a needs: job inside the release-please workflows rather than on a
standalone release: published trigger, because release-please creates the
release with GITHUB_TOKEN and GitHub never starts new workflow runs from
token-generated events -- a standalone trigger would never fire. Shared as
a reusable workflow_call so the stable and beta channels stay in sync.
Best-effort: continue-on-error + fallback mean it can never break a release.
Requires GitHub Models enabled for the org; until then the fallback is used.
Branch protection on `main` + `stable` lists `upgrade-from-bootstrap`
and `fresh-install` as REQUIRED checks. The producing workflows had
`paths:` filters in their `pull_request` triggers, so they correctly
skipped on PRs that didn't touch migrations / package.json. But a
skipped workflow doesn't satisfy a required check — it leaves the
status "missing", which blocks merge on every unrelated PR.
Concretely surfaced on PR #692 (security bumps): all 12 visible checks
were green, but the merge button was blocked because the two
path-filtered workflows skipped and their required-check names never
reported.
This PR drops the `paths:` filter from both workflows so they always
fire on PRs against `main` + `stable`. Costs:
- `schema-drift` (`upgrade-from-bootstrap`): ~75 s per PR (Postgres
service boot + migrate:safe run + schema assertion).
- `install-smoke` (`fresh-install`): ~2 min per PR (full Docker
Compose boot + login).
Both are buying unconditional safety nets on the install + migration
paths, which is what the required-check gate is supposed to model.
Also fixes the trigger branch list while in the file: `[main, beta]`
→ `[main, stable]`, completing the post-#669 rename for these two
workflows that were missed in PR #686.
## What this does NOT fix
`GitGuardian Security Checks` is the third required check that's
currently missing on PRs — but that's a separate problem. The
GitGuardian GitHub App was installed at the user-account level
(`the-luap`) before the org transfer and didn't move with the repo.
Re-installing it on the org via the GitHub Marketplace is a UI step
the maintainer needs to do; can't be done via API.
@Luca-Timo is on main's review-bypass list so he can self-merge small
bugfixes without waiting for a maintainer review. The bypass list alone
is binary (he can merge anything), so this adds a complementary required
status check that fails when a bypass user's PR exceeds a configured
line-count threshold — blocking merge for genuine features while leaving
small bugfixes flowing.
How it works:
- Trigger: pull_request_target (so the workflow runs in the base repo's
context with permissions to write a check status — script never
executes PR code, so fork-PR-attack-safe).
- For PRs authored by a bypass user (default: @Luca-Timo):
- linesChanged = additions + deletions
- If ≤ LINE_LIMIT (300): check = success → bypass works → self-merge OK
- If > LINE_LIMIT: check = failure → required-check gate blocks merge
regardless of bypass; needs a maintainer review.
- For everyone else: check = success ("not applicable"). They go through
the normal review path and are unaffected.
Both constants (LINE_LIMIT, BYPASS_USERS) are at the top of the workflow
for easy tuning.
After this lands on main, a separate API step adds 'bypass-size-gate' to
the main branch's required_status_checks list so the gate is actually
enforced. Until that's in place the check runs but doesn't block.
After the org move + branch rename (#669):
beta → main (active development)
main → stable (curated release channel)
This PR rewires the workflows that referenced the old branch names so
release-please and the Docker build target the right channels.
## Workflow changes
### `.github/workflows/docker-build.yml`
- **Push triggers**: `[main, beta]` → `[main, stable]` (both `push.branches`
and `pull_request.branches`). `beta` no longer exists; `stable` is the
curated channel that should also produce builds.
- **`is_prerelease` detection**: pre-release context was decided by
`refs/heads/beta`; now decided by `refs/heads/main` (active dev →
prerelease, `-beta.N` version suffix unchanged).
- **`:latest` + `:stable` tagging**: were gated on `{{is_default_branch}}`
(which used to be `main` = stable channel). Default branch is now `main`
= active dev, so the implicit gate would have aliased `:latest` to dev.
Both tags now explicitly gate on `refs/heads/stable` OR a non-prerelease
release tag.
- **`:beta` tag**: REMOVED. Active-dev pulls are `:main` (auto-generated
by `type=ref,event=branch`). The pre-rename `:beta` tag remains frozen
at its last build under Option B / #669 — operators are expected to
update to `:main` or pin to a versioned tag.
### `.github/workflows/release-please.yml`
- `branches: [main]` → `branches: [stable]`. This is the **stable**
release-please workflow (uses `release-please-config.json`); after the
rename, the stable channel lives on the `stable` branch.
### `.github/workflows/release-please-beta.yml`
- `branches: [beta]` → `branches: [main]`.
- `target-branch: beta` → `target-branch: main`.
- This is the **pre-release** release-please workflow (uses
`release-please-config-beta.json`, `prerelease: true`); after the rename,
pre-releases are cut from the new `main` (active dev). The version-suffix
scheme stays `-beta.N` so existing operator pins keep working.
## RELEASING.md
Rewrote the TL;DR, "How a stable release is cut", and hotfix path to
reference the new branch names. Added a one-line "branch model background"
note pointing at #669 so future maintainers know why `main` means active
dev (the opposite of what some projects use). Filename conventions:
`release/X.Y.Z-merge-from-main` (was `…-from-beta`); promotion PR title
`promote main → stable as vX.Y.Z` (was `promote beta → main`).
## Why combined with PR A's content as a single PR
Originally planned as two PRs (B = workflow triggers, C = release-please
reconfigure). Splitting wasn't worth it: the configs are branch-agnostic
(`release-please-config.json` and `release-please-config-beta.json` don't
mention branch names internally), and not bundling them meant a window
where the stable release-please workflow would fire on pushes to the new
`main` (active dev) — exactly the wrong place. Single PR closes that gap.
## Versioning scheme — kept
No version-scheme decision needed. The `-beta.N` suffix on pre-release
versions is preserved (existing operator pins like `v3.71.3-beta.0` keep
working). If a `v4.0.0-pre.N`-style reset is desired later, that's a
separate PR with explicit operator-comms attached.
Repo transferred from the-luap/picpeak → PicPeak/picpeak. Docker images
publish to ghcr.io/picpeak/picpeak/{backend,frontend} (lowercase, per the
GHCR canonical form computed by docker-build.yml's `${GITHUB_REPOSITORY,,}`).
Sweep covers:
- docker-compose.production.yml + Dockerfiles → new image registry path
- README, CONTRIBUTING, SECURITY, SIMPLE_SETUP, scripts/picpeak-setup.sh
→ new GitHub URLs
- Update-check / release-notes services (updateCheckService,
environmentService, updateNotificationService, adminSystem,
UpdateNotification, githubReleaseUrl) → GitHub API + tag URLs use the
canonical PicPeak/picpeak path
- Issue templates + README-DOCKER + workflow README → updated package URLs
- One commit-context comment in migrations/090 + customerAccountsService
CHANGELOG.md is intentionally untouched (historical release entries are
immutable; GitHub auto-redirects the old URLs indefinitely).
CLAUDE.md keeps the bare `(the-luap)` reference — that's the maintainer's
personal handle, not a repo URL.
22 files, 48/48 line swaps (every change is a 1:1 URL replacement).
The frontend/backend image builds + pushes succeed, then the final
'exporting to GitHub Actions Cache' step intermittently fails with
'error writing layer blob: not_found' (a known flaky type=gha cache backend
issue), failing the whole job. Add ignore-error=true to every cache-to so a
cache-write hiccup can't break an otherwise-successful, already-pushed build.
The AdminHeader "Clear All" notifications button has been 404'ing for
a while: frontend `notifications.service.ts` calls
`DELETE /admin/notifications/clear-all`, backend only defined
`DELETE /admin/notifications/clear-old`.
The /clear-old route was misleadingly named anyway — it tried to
delete read OR >30-days-old rows, then had a fallback that nuked
EVERY row when nothing matched. Both the frontend and the existing
test expect a simple Clear All shape, so just rename to /clear-all,
drop the tiered logic, and return the plain
`{ message, deletedCount }` payload the test asserts on.
The test (adminNotifications.test.js) was hiding the breakage —
it was on CI's --testPathIgnorePatterns ignore list and so never
ran. Two reasons it failed locally before this fix:
1. Route path mismatch (the actual #597 bug).
2. The mock only stubbed adminAuth — requirePermission lives in
its own middleware module and ran for real, 403'ing before
the handler. Add a passthrough mock for that too.
With both fixed, the test passes. Drop adminNotifications from the
CI ignore list so future regressions in this route fail loudly
instead of going to ground.
The suites already existed (538 backend tests, 40 frontend tests, with
solid CRM coverage on quoteService/contractService/invoiceService/
customerHoursService/eventService.calendar) but no CI workflow invoked
them. Wire both into a single Tests workflow that triggers on any push
or PR to main/beta.
Six backend suites are excluded — they fail on upstream/beta too
(supertest fixture + knex mock chain issues unrelated to CRM). The
explicit ignore pattern keeps the workflow green on day 1; each
excluded suite is listed inline as test-infra debt to fix individually.
Backend job pins SKIP_S3_TESTS=true (the same default the test setup
file applies) so the backup-service integration doesn't try a real S3
round-trip when no MinIO is provisioned.
First CI run failed at the precondition check because the SQL `CASE WHEN
to_regclass(...) IS NULL THEN 0 ELSE (SELECT count(*) FROM migrations)`
expression doesn't short-circuit at parse time — Postgres parses the
subquery against `migrations` even when the outer guard would skip it,
fails the run with "relation 'migrations' does not exist".
initializeDatabase() doesn't create the `migrations` tracking table —
that's the migrate:safe runner's responsibility — so in the recovery
scenario the table genuinely doesn't exist yet. Both "absent table" and
"present but empty table" are valid recovery states.
Split the check into two shell steps: to_regclass first, then count only
if the table exists. Avoids the parse-time subquery error and accepts
either state.
Refined from the original #530 framing after a dry-run uncovered that the
"bootstrap vs migration chain" diff produces mostly noise — most of the
~200 lines of difference are expected (migrations add new tables and
columns over time). initializeDatabase() isn't a parallel path that
diverges from migrations; it's invoked by migration 001 itself, so every
normal install/upgrade runs both.
The genuine drift hazard surfaced during the dry-run: a DB with the
modern bootstrap tables but an empty `migrations` table (which happens
when a backup was restored that lost the migrations table, or someone
invoked initializeDatabase() outside the runner, or the DB was moved
between systems without copying the migrations row) fails to upgrade.
Failure mode:
1. detectExistingSchema sees the bootstrap tables + empty migrations,
treats it as an "existing deployment".
2. Runs the legacy chain first.
3. legacy/008 renames email_templates.subject → subject_en.
4. core/029 (later in the chain) inserts email templates referencing
the pre-rename `subject` column.
5. Postgres rejects: column "subject" doesn't exist; subject_en is
NOT NULL with no default.
Fresh installs avoid this because they only run core/* (and core/059
handles the rename AFTER core/029 has inserted). Real legacy upgrades
avoid it because their migrations table already records legacy/008–028
as applied historically.
Fix in detectExistingSchema:
- Detect the modern bootstrap fingerprint (photo_categories + cms_pages
both present, which initializeDatabase produces as part of the
consolidated post-004-era bootstrap).
- When matched, enumerate every file in migrations/legacy/ and mark
each as applied. This puts the recovery state on the same code path
fresh installs use — only core migrations run, in core order.
- Real legacy upgrades that already have entries in the migrations
table hit no-op markings (markMigrationAsApplied skips duplicates),
so their behaviour is unchanged.
New CI workflow (`.github/workflows/schema-drift.yml`):
- Boots fresh postgres.
- Seeds via `node -e \"require('./src/database/db').initializeDatabase()\"`
— reproduces the recovery state in one line.
- Runs `npm run migrate:safe`.
- Asserts: precondition (bootstrap fingerprint + empty migrations
table), migrate:safe exits 0, final schema has ≥40 tables (soft floor,
not exact pin so future migrations don't force workflow edits),
legacy migrations marked applied (confirms the fingerprint check
actually fired vs. the chain silently bailing).
- Triggers only on PRs that touch backend/migrations/**,
src/database/db.js, knexfile.js, or this workflow.
Manually verified end-to-end before this commit:
Before fix: migrate:safe dies at core/029 with NOT NULL violation
on email_templates.subject_en (17/48 tables present).
After fix: 82 migrations applied + 27 marked applied = 109 total,
final state has all 48 tables matching fresh-install.
Issue body in #530 has been updated to match this refined scope.
Refs: #530, #484, #519
Alpine ships BusyBox ps (no -p PID, no pgrep), which failed CI on the
first run of this workflow with "ps: unrecognized option: p". Replace
the pgrep-then-ps chain with `ps -o user,comm | awk '$2=="node"'`
which works on both BusyBox (Alpine, in the container) and procps
(the GitHub runner host, though we don't use it here).
The fresh-install restart loop reported by @MrGabri (and confirmed by
@AloePacci with the user:0:0 workaround) had a clear root cause:
- Dockerfile pinned USER nodejs (UID 1001) before the entrypoint
ran, so the existing chown branch in init-production.sh:13 was
dead code.
- wait-for-db.sh (the actual entrypoint, not init-production.sh)
silently swallowed mkdir/EACCES on bind mounts with || true,
then a downstream migration error surfaced as the visible failure.
- Net effect on a typical Linux host where the bind-mount dir is
owned by UID 1000: container can't write, exits non-zero,
restarts forever with no clear error.
Switch to the standard Docker drop-privileges pattern:
1. Install su-exec, drop `USER nodejs` from the Dockerfile —
container now starts as root.
2. wait-for-db.sh: if running as root, chown /app/storage,
/app/data, /app/logs to nodejs and re-exec self via
su-exec nodejs:nodejs. App still ends up running as UID 1001.
3. Preflight check for non-root invocations (compose `user:`
overrides): verify the bind mounts are actually writable
before continuing. If not, exit 1 immediately with an
actionable error pointing at the docs — no more silent
restart loops.
Also:
- Delete backend/init-production.sh. It was an orphan — no caller
in the Dockerfile, compose, or anywhere else. Its chown logic
looked authoritative enough that @MrGabri ran it manually trying
to debug, which is what finally surfaced the EACCES.
- docker-compose.yml: drop user: + PUID/PGID env. The pattern-B
UID-matching workaround they implemented is obsolete now that
pattern A (root-then-drop) is in place.
- .env.example + README: drop PUID/PGID documentation.
- Add fresh-install smoke test workflow. Boots backend + postgres
against bind mounts owned by UID 1000 (the GitHub runner UID,
and the common-mismatch case on Linux hosts) and verifies:
+ container reaches healthy without restart-looping
+ chown happened (dirs now owned by 1001 inside the container)
+ node runs as nodejs, not root (su-exec drop worked)
+ /health returns status:ok
+ with --user 5005:5005 + unwritable mounts, preflight exits
loud with the expected error string
Verified locally end-to-end against a fresh Postgres + UID-501-owned
bind mount: backend reaches healthy in ~20s, chown applied, node
runs as nodejs, no restart loop. Docs in picpeak-docs cover the new
behavior + a Troubleshooting section for the install-path bugs
fixed in #484/#494/#511/#488.
Refs: #484
PR #477 moved Trivy from the merge-* job into the per-arch build-*
matrix scanning by digest. The amd64 leg works; the arm64 leg
crashes with:
remote error: no child with platform linux/amd64 in index
ghcr.io/.../<image>@sha256:<digest>
Root cause: docker/build-push-action wraps every push in an OCI
index — the actual image manifest sits next to a SLSA provenance
attestation manifest as siblings under the digest. Trivy's remote
backend defaults to linux/amd64 when resolving an index, so:
- amd64 leg → looks for amd64 child → finds the amd64 image → ok.
- arm64 leg → looks for amd64 child → finds NO amd64 child
(the only platform child is arm64) → fails.
Fix: set TRIVY_PLATFORM = ${{ matrix.platform }} on each leg's
Trivy step. Each scanner then asks for its own arch and finds it.
SLSA provenance attestation stays attached to the per-arch images
— a real win for supply-chain visibility we'd lose if we'd
disabled provenance instead.
amd64 was the only thing keeping CI partly green; this restores
full green across both legs without touching the build artifact
shape.
Initial pinning shipped a tag that doesn't exist in the
aquasecurity/trivy-action repo. Workflow run failed with:
Unable to resolve action 'aquasecurity/trivy-action@0.28.0',
unable to find version '0.28.0'
The repo's tags use a v prefix (v0.36.0, v0.35.0, …). Bumping
both occurrences (build-backend and build-frontend matrix jobs)
to v0.36.0, which is the latest stable as of 2026-04-22.
Resolves the intermittent "no child with platform linux/amd64 in
index" failure on the merge-backend job — and fixes the same latent
bug on merge-frontend before it surfaces.
Two compounding root causes per Luca's diagnosis:
1. aquasecurity/trivy-action@master was unpinned, so the action and
its bundled Trivy binary float on every CI run. A green build
could flip red overnight without a single repo change.
2. Trivy was asked to scan a multi-platform OCI index by tag (the
merge-* jobs ran AFTER manifest creation). Its remote resolver
cannot reliably pick the right per-arch child out of an index
reference — it needs a single-platform reference (digest, or a
--platform flag).
Fix:
- Move the Trivy + upload-sarif steps OUT of merge-backend /
merge-frontend and INTO the per-arch build-backend / build-frontend
matrix jobs. Each leg scans the image it just pushed by its
sha256 digest (`...@${{ steps.build.outputs.digest }}`), which is
always single-platform by construction.
- Pin aquasecurity/trivy-action@0.28.0 (was @master).
- Distinct SARIF category per arch
(`backend-vulnerabilities-linux-amd64`, …-arm64) so an
amd64-only finding in a base layer doesn't get masked by the
arm64 scan in the Security tab.
- Move security-events: write down to the build-* jobs (where the
scan now runs) and remove it from the merge-* jobs (which only
publish the manifest now).
Out of scope: flipping `exit-code: '1'` to actually gate CI on
findings. Worth doing as a separate follow-up after an audit pass —
landing it here would surprise beta with a red build for any
pre-existing CRITICAL/HIGH in current images. Inline TODO in the
workflow notes the deferral.
Triage of an external SAST/SCA scan run on 2026-05-06. Most loud findings
were already resolved by PR #412 (the 18-CVE backport); this PR addresses
the residual real items:
* Drop unused `handlebars` from backend deps. The runtime require was
removed in PR #367 (#367) but the package.json line stayed. handlebars
was the source of two flagged criticals (CVE-2026-33937 RCE,
GHSA-2w6w-674q-4c4q AST injection) plus 8 highs — all now gone.
* `npm audit fix` on backend + frontend. Bumps transitive picomatch,
flatted, postcss, brace-expansion via lockfile, and direct dompurify,
lodash, vite, i18next-http-backend within their existing semver ranges.
Both audits now report 0 vulnerabilities.
* Add `event.origin === window.location.origin` check to the THEME_PREVIEW
message listener in PreviewPage. The branding page posts from the same
origin, so nothing legitimate is rejected; without the check, any third
party that window.open()'d the preview could push arbitrary
branding/theme payloads (semgrep
insufficient-postmessage-origin-validation).
* nginx: `proxy_hide_header` for X-Frame-Options, X-Content-Type-Options,
Referrer-Policy, Content-Security-Policy, Permissions-Policy,
Strict-Transport-Security at server level. nginx adds these itself, but
helmet on the backend was also emitting them — clients were seeing
duplicates (testssl flagged "Multiple X-Frame-Options / CSP /
Permissions-Policy / Referrer-Policy headers" on the live origin).
Single source of truth now.
* Dockerfile hardening (checkov):
- HEALTHCHECK on backend/Dockerfile, backend/Dockerfile.dev,
frontend/Dockerfile.dev. Frontend production Dockerfile already had
one.
- USER node in frontend/Dockerfile.dev (was running as root).
* GitHub Actions docker-build.yml: explicit top-level
`permissions: contents: read`. Per-job blocks already declare
`packages: write` where needed; this stops future steps from
inheriting unintended privileges (CKV2_GHA_1).
Backend npm audit: 4 vulns -> 0.
Frontend npm audit: 6 vulns -> 0.
Backend unit tests: 13 suites, 131/132 passing (1 pre-existing skip).
Frontend type-check + lint: clean.
The pre-existing integration-test failures (live DB / S3 required) and
the ThemeCustomizerEnhanced QueryClientProvider failures are unrelated
and reproduce on origin/beta without these changes.
Remove sync-versions job that fails on protected branches.
Instead, use Release Please's extra-files feature to update
package.json versions as part of the release PR.
QEMU emulation of ARM64 on x86 GitHub runners is too slow and
unreliable for npm operations, causing builds to hang or crash
with "Illegal instruction" errors.
Changed platform detection logic to:
- Tagged releases (v*.*.*): Build both amd64 and arm64
- All other builds (branches, PRs): Build amd64 only
This ensures fast CI feedback during development while still
providing multi-arch images for production releases.
Sharp library native binaries cause QEMU 'Illegal instruction' errors during
ARM64 emulation. This change builds only amd64 for PR checks (faster, reliable)
while maintaining multi-arch (amd64+arm64) builds for main/develop/tags.
The workflow was generating invalid Docker tags with format ':-3b251d7'
due to empty branch names in PR contexts.
Problem:
- Tag config: type=sha,prefix={{branch}}-,format=short
- For PRs: {{branch}} is empty → results in ':-3b251d7' (invalid)
- Docker doesn't allow tags starting with hyphen
Solution:
- Changed to: type=sha,format=short
- Now generates: '3b251d7' (valid) without branch prefix
- Works correctly for PRs, branches, and tags
Valid tag examples now:
- PRs: pr-44, 3b251d7
- Branches: main, 3b251d7
- Tags: v1.0.0, 1.0, 1, 3b251d7
The publish-manifest job was failing because it tried to create manifests
from non-existent architecture-specific tags (latest-amd64, latest-arm64).
docker/build-push-action@v5 already creates multi-arch manifests automatically
when building for multiple platforms, making this job redundant.
The workflow now correctly builds and pushes multi-arch images in a single
step with proper manifest lists included.
🤖 Generated with [Claude Code](https://claude.ai/code)
Co-Authored-By: Claude <noreply@anthropic.com>
Paul Nothaft
paul