Addresses PR #404 review findings F2/F3/F4 on issue #186:
- F3 (fail-closed): ship EPPP_SESSION_SECRET placeholder as `change-me`
(9 chars), shorter than the schema's 32-character minimum, so an
unedited `cp .env.example .env` is rejected at startup instead of
booting with a publicly known secret.
- F2 (gitleaks-clean): build the secret-shaped mutation-probe literal at
runtime from short non-secret fragments; the branch no longer embeds a
secret-shaped literal in the test source.
- F4 (masked messages): assertion messages mask/truncate values that come
from the template instead of echoing the raw string.
- test: add a fail-closed length test for the EPPP_SESSION_SECRET
placeholder (< 32 chars); keep mutation probes non-vacuous.
Extend the env-adapter suite to the issue's reworked acceptance criteria:
- static assertions: the adapter resolves HOST via resolveHost (hostname/IP at
the adapter boundary) and the server passes config.host to server.listen
- deterministic boundary probe: valid HOST forms (IPv4/IPv6/hostname) pass,
invalid HOST forms throw ConfigStartupError naming host
- boot probes: HOST=127.0.0.1 binds loopback only (no answer on a
non-loopback interface) with the startup log reflecting the actual bind;
an invalid HOST exits non-zero naming the field without echoing the raw
value
- mutation probes: bypassing resolveHost or dropping config.host from
server.listen both fail
- config-startup-error: update the order-asserion mutation probe for the new
server.listen(config.port, config.host, ...) signature
- tests/config-startup-error.test.mjs: static assertions on the committed
startup-error module, the package boundary, the server wiring (validation
before bind), the compose secret and the Dockerfile shipping, each backed
by mutation probes; the deterministic probes execute the issue's test plan
("start with a missing required field and confirm the error names it") —
the compiled boundary throws MissingRequiredSettingError naming
sessionSecret, and booting the committed server without EPPP_SESSION_SECRET
exits non-zero naming the field while a valid secret boots to /health 200
- health-endpoint/app-readiness boot probes: provide a valid
EPPP_SESSION_SECRET (the required setting is validated at startup)
- ci.yml: new config-startup-error job (builds config + database-postgres,
runs the suite); app-readiness job now builds the config package too
- .gitignore: transient .config-startup-probe-*.mjs files
tests/config-schema.test.mjs covers both acceptance criteria: the schema
is defined with TypeBox/Ajv (static assertions on the committed package —
golden-tuple exact pins, Type.Object schema, Ajv compile, boundary
re-exports — each backed by a mutation probe proving non-vacuity) and the
schema covers the validated config fields (host, port, databaseUrl,
sessionSecret with their constraints). The deterministic probe executes
the issue's test plan — 'validate a full config against the TypeBox/Ajv
schema' — against the committed schema through Ajv via Node type
stripping (no build step), plus the negative cases (missing required field
naming sessionSecret, secret too short, unknown property, port bounds, and
empty databaseUrl); when the package is built (as in the CI job) it also
exercises the compiled validateConfig boundary exactly as the later
adapter will consume it.
Adds packages/config (@personal-blog/config) — the EPPP configuration
service foundation. The package defines the configuration schema with
TypeBox (configSchema: host, port, databaseUrl, sessionSecret — the
validated config fields, golden-tuple pins @sinclair/typebox@0.34.52 and
ajv@8.20.0) and compiles it with Ajv (validateConfig). The environment
adapter (T04), field-specific startup errors (T02) and secret redaction
(T03) build on this boundary in later tasks; nothing reads process.env yet.
Wiring for the new workspace package: lockfile importer + resolved
typebox/ajv tree, apps/server/Dockerfile manifest copy (frozen in-image
install must match the lockfile importers), config-schema CI job, package
set fixtures (workspace-layout, workspace-config, strict-tsconfig,
typescript-pin), probe-file gitignore entry.
Static assertions + mutation probes on the committed runner source, a
deterministic stub-pool behavioral probe (intentionally failing migration
fixture -> structured diagnostic naming the failing migration, apply and
record phases), a docker-gated real-stack probe against a real database
(the issue's test plan), and CI enforcement via the additive
database-postgres-diagnostic job.
release() previously returned the connection to the pool in a finally even
when the pg_advisory_unlock statement failed, so a pooled connection could be
reused while its session still held the migration advisory lock - the next
borrower would block every other runner (reviewer finding F4). On unlock
failure the connection is now destroyed (client.release(error) removes the
client from the pool, ending the session and its lock); the plain
client.release() is kept only on the success path. Locked in by a static
assertion, a mutation probe, and a deterministic stub-pool behavioral probe
of the committed release() control flow.
Security-review finding F2: two concurrent acquire() calls on the same
MigrationLock instance could each check out a connection; the second
pg_advisory_lock would overwrite this.client, leaking the first locked
connection until session end.
acquire() now memoizes the in-flight acquire in acquireInFlight and
returns it on re-entry, so exactly one connection is checked out and no
locked connection leaks. The memo is cleared once the acquire settles.
tryAcquire()/release() paths unchanged.
Locked in by:
- static criterion test: acquireInFlight field, re-entry guard returns
the in-flight acquire, memo cleared on settle
- mutation probe: removing the re-entry guard fails the criterion
- real-stack probe: two concurrent acquire() calls on one instance leave
pool.totalCount at 1 (exactly one connection), the lock granted once,
nothing left after release; probe fails (hangs) on the pre-fix code
- CI job comment updated to reflect the re-entrancy criterion
Static assertions on the committed ledger source (idempotent table DDL,
parameterized idempotent record, has/applied queries, driver-boundary
re-export, CI enforcement) with mutation probes proving non-vacuousness;
docker-gated real-stack probe migrates an empty database (isolated compose
project + host port) and confirms the ledger exists, the applied migrations
are recorded, and a re-run records nothing twice. New additive
database-postgres-ledger CI job gates the criterion on every PR.
Review finding on PR #391: packages/database-postgres pinned pg@8.23.0 and
kysely@0.29.5, but the architecture doc's golden tuple (Technology-Stack
section 5.2 / section 7) pins pg@8.22.0 and Kysely@0.29.4. Reproducibility
requires the exact documented versions.
- packages/database-postgres/package.json: pg 8.23.0 -> 8.22.0,
kysely 0.29.5 -> 0.29.4, @types/pg 8.23.1 -> 8.21.0 (no 8.22.x of
@types/pg is published; 8.21.0 is the closest matching release, types
for the immediately preceding pg minor)
- pnpm-lock.yaml: regenerated with pnpm 11.23.0 (Node 24); the resolved
pg dependency tree is unchanged apart from the driver version itself
- tests/database-postgres-imports.test.mjs: exact-pin assertions updated
to the corrected versions, with a comment noting the @types/pg choice
- database-postgres-imports.test.mjs: static scan of every workspace package
source proves pg/kysely import specifiers resolve only to
packages/database-postgres; owner manifest pins the driver and no other
package declares it; mutation probes prove the scan catches a driver import
injected into apps/server/src/index.ts; comment-stripping and specifier
matcher unit probes; CI-enforcement assertion
- workspace-layout / workspace-config / strict-tsconfig / typescript-pin:
package-set fixtures updated to include packages/database-postgres
- docs/development/non-container.md: workspace package table and build
expectations updated for the new package
- compose-config.test.mjs: assertDbService requires the pinned
postgres:18.6-bookworm image; new docker-gated real-stack probe starts the
stack and asserts SHOW server_version exposes 18.6; mutation probe proves
reverting to a floating major tag fails the criterion; parser probe updated
- build-targets.test.mjs: db-service mutation fixture updated to the pinned
image tag
The Docker-gated probe only planted a root-level .env.t08-* marker, which no
Dockerfile COPY instruction ever copies — so it could not observe a nested
build-context leak in the image layers. Plant additional marker files at
nested paths the Dockerfile's COPY apps/server apps/server would sweep into
the build-stage image (apps/server/.env.t08-*, apps/server/secrets/t08-*.pem)
so the end-to-end scan actually verifies the 'any depth' exclusion
guarantee, not just the root form.
Resolve the security review of #389 (findings 1-4):
- .dockerignore: every env/credential pattern is now **/-prefixed
(**/.env, **/.env.*, **/node_modules, **/.npmrc, ..., **/secrets,
**/*.pem, **/*.key, ...) and the redundant 'secrets/' line is dropped.
Docker's matcher (moby/patternmatcher) anchors slash-less patterns to
the context root, so the bare forms excluded nothing under apps/server/;
**/ matches the root AND any nested depth. (finding 1, 3)
- tests/secrets-not-embedded.test.mjs: the dockerignore matcher is now a
faithful port of moby/patternmatcher (filepath.Clean + anchored full-path
match + parent-directory propagation), not gitignore basename semantics;
asserts nested example paths (apps/server/.npmrc, config/server.key,
apps/server/secrets/...) are excluded; requires no redundant equivalent
patterns verbatim; adds mutation probes for bare-pattern and
duplicate-pattern regressions. (finding 2, 3)
- apps/server/Dockerfile + compose.yaml: guarantee restated precisely
(credential files excluded at the context root AND at any depth).
- .gitea/workflows/ci.yml: new job runs
'node --test tests/secrets-not-embedded.test.mjs' on every PR; the
docker-gated layer-scan probe runs where a daemon exists, skips cleanly
otherwise. (finding 4)
- tests/compose-config.test.mjs: .dockerignore presence list updated to the
**/-prefixed forms (node_modules, .env).
Tested: secrets suite 16 tests -> 15 pass / 1 docker-gated skip / 0 fail;
full suite 101 pass / 12 fail / 8 skip, failures identical to clean main
(env-dependent pnpm/Node-24 suites); matcher port verified against the
moby/patternmatcher evidence table.
tests/secrets-not-embedded.test.mjs: static assertions that the Dockerfile
embeds no secrets (no secret-bearing ARG/ENV, no secret-path or blanket COPY)
and .dockerignore excludes env/credential files; non-vacuous mutation probes
for every assertion; Docker-gated probe that builds the image with a marker
env file in the context and scans every layer + image config for secret
values.
tests/non-root-user.test.mjs locks in both acceptance criteria: a static
assertion that the Dockerfile runtime stage declares a non-root USER (not
root/uid 0, 'USER node' exactly), non-vacuous mutation probes, and a
Docker-gated real-stack probe that starts the stack and asserts 'id -u' and
'id -un' inside the running app container report a non-root user, with the
health endpoint still answering as a regression guard.
compose-config: assert the db service mounts the named db-data volume at
the PostgreSQL data directory and the top-level volumes map declares it;
non-vacuous mutation probes (missing mount, missing volume declaration,
wrong mount target all fail); Docker-gated real-stack probe writes a
fixture row and asserts it survives `docker compose restart` (restart)
and `docker compose down` + `up -d` (recreate), cleaned up with
`docker compose down -v`.
Switch the app image from node:24-alpine to node:24.19.0-bookworm-slim in
both build and runtime stages (Technology-Stack 5.4: glibc Debian base
required because argon2 is a native dependency; musl/Alpine causes
native-module build surprises), and the db image from postgres:16-alpine
to postgres:18-bookworm (Technology-Stack 5.2/5.4/6.2 + golden tuple 7
pin PostgreSQL 18.6; 18-bookworm is the 18.x line on Debian bookworm).
Update tests/compose-config.test.mjs so the committed assertions lock in
the corrected image bases (db image, Dockerfile build/runtime stages,
parser probe).
Adds a declarative dependency-boundary rule (dependency-boundaries.json)
classifying workspace packages into apps/packages/extensions groups and
forbidding packages/* (core) from importing extensions/* (concrete
extensions); core depends only on extension contracts, never concrete
extensions.
Adds tests/architecture-import.test.mjs (node:test, zero new dependencies,
lockfile untouched) that loads the rule, walks every workspace package's
source and resolves each import/export/require specifier (bare workspace
names, relative paths, dynamic import(), require(), export-from) to a
group, failing on any forbidden core -> extension edge. Comment-aware
scanning avoids false positives; line numbers in violation reports map to
the original source. Synthetic negative cases prove detection (bare name,
relative path, export-from, dynamic import, require), while the current
compliant graph passes with zero violations.
Verified: 10/10 tests pass; injecting a real core -> extension import makes
the integration test fail with a per-file/line violation report; pnpm 11.23.0
install --frozen-lockfile passes unchanged (CI frozen-install job stays green).
Closes#157
Extends the newsletter suite with two boundary cases: redirects (301/302/307/
308) must be treated as failures with the user-safe error, and the POST must
carry no credential of any kind — no api-key/auth-token/cookie headers, and no
token/secret in the body or URL.
Rework PR #15 per security review SEC-14-R1: the client no longer carries
an API token. js/newsletter-config.js ships only the non-secret endpoint,
enforced https-only at config load time via validateEndpoint() (mirroring
the protocol allowlist in js/reading-list.js); js/newsletter.js POSTs
email-only with no Authorization header. Failure paths keep the single
user-safe message that never leaks token, endpoint, status, or raw body;
success still shows the confirmation. CI gains a gitleaks step that fails
on any secret hit; README documents the server-side token, the residual
signup-abuse risk, and the authoritative server-side validation follow-up.
Covers the issue test plan: form renders and posts to the endpoint; token is
read from config, never hardcoded in source; a failed-token response shows a
user-safe error without leaking the secret; confirmation on success.
All reading list links point at external http(s) URLs, so each rendered
anchor now carries rel="noopener noreferrer" as a hardening best
practice. Renderer test updated for the new attribute and asserts every
rendered link carries it.
Covers: page wiring and nav reachability, data-file integrity, grouped
rendering, valid hrefs, optional notes, HTML escaping, invalid-entry
skipping, data-only extensibility, and a 500-entry fixture (counts,
grouping, and a generous render-time bound).
implementer
kpcto
bot-implementer