DC-040: claim for Hermes
CI / Test & Lint (push) Has been cancelled
CI / Security audit (push) Has been cancelled

This commit is contained in:
Hermes
2026-07-13 15:03:49 -07:00
parent 5f30fbf1ca
commit b13960fa9a
+2 -2
View File
@@ -204,8 +204,8 @@
- **result:** Found **and fixed** the antipattern across 10 modules in `src/`. 13 distinct `path.join(__dirname, 'foo.json')` defaults (plus the `__dirname` based `LOG_DIR`/`ERROR_LOG_FILE`) all wrote runtime state into the source tree, surviving in dev but landing in the image layer in production. Centralised resolution in `platformPaths.dataDir` (derived from `SERVICES_FILE` env when set, else `path.dirname(servicesFile)`); the 10 modules now route their `*-config.json` / `*-history.json` / `.port-locks` / `audit-log.json` / `error.log` / `.license-secret` / `.license-counter` defaults through it, preserving per-file env-var overrides. `crypto-utils.js` and `credential-manager.js` already had a multi-candidate resolver; collapsed them to a single `platformPaths.dataDir` lookup. The `host-registry` / `event-store` / `event-workers` `dataDir || path.join(__dirname, '../../data')` pattern simplified — the legacy fallback is unreachable now that `services.json` lives at `dataDir`. Also fixed a **real production bug found mid-audit**: `audit-logger.js` defaulted `AUDIT_LOG_FILE` to `/app/src/security/audit-log.json` and `logging.js` defaulted `LOG_DIR` to `__dirname` (i.e. `/app/src/utils/`), so every error-log/audit-log write was landing in the image layer — a fresh container recreate would have wiped the entire audit log. Now both flow through `dataDir` which the start.sh bind mount already points at `/app/data`. Drive-by: removed unused `readline` import in `event-workers.js`. Also fixed a **test gap** in `__tests__/public-routes-drift.test.js`: `routes/security.js` was missing from the direct-mounts list, so the `/api/v1/security/events/ingest` and `/api/v1/security/events/batch` PUBLIC_ROUTES entries (added by DC-044) were flagged as stale. Added it with `/security` prefix mapping. **Pre-existing files on the running container (`audit-log.json` 319KB, `container-stats*.json` 186MB, `workflow-history.json` 269KB, `audit-log.json` etc.) are still in the image layer** — those are lost on next recreate unless a one-time migration step runs; out of scope for this fix but flagged for a follow-up. **Tests: 1214/1214 pass, +0 failures. ESLint: 146 warnings + 4 errors — identical to baseline (no new warnings/errors introduced).** Docker container does NOT need rebuilding: the affected code paths are evaluated at boot, and `dashcaddy-api/data/` is the existing bind mount — the new defaults resolve to the same path the container already uses via env vars (`CREDENTIALS_FILE=/app/data/credentials.json`, `ENCRYPTION_KEY_FILE=/app/data/.encryption-key`, etc.), and the env vars take precedence. Self-updater picks it up on the next release bump. - **result:** Found **and fixed** the antipattern across 10 modules in `src/`. 13 distinct `path.join(__dirname, 'foo.json')` defaults (plus the `__dirname` based `LOG_DIR`/`ERROR_LOG_FILE`) all wrote runtime state into the source tree, surviving in dev but landing in the image layer in production. Centralised resolution in `platformPaths.dataDir` (derived from `SERVICES_FILE` env when set, else `path.dirname(servicesFile)`); the 10 modules now route their `*-config.json` / `*-history.json` / `.port-locks` / `audit-log.json` / `error.log` / `.license-secret` / `.license-counter` defaults through it, preserving per-file env-var overrides. `crypto-utils.js` and `credential-manager.js` already had a multi-candidate resolver; collapsed them to a single `platformPaths.dataDir` lookup. The `host-registry` / `event-store` / `event-workers` `dataDir || path.join(__dirname, '../../data')` pattern simplified — the legacy fallback is unreachable now that `services.json` lives at `dataDir`. Also fixed a **real production bug found mid-audit**: `audit-logger.js` defaulted `AUDIT_LOG_FILE` to `/app/src/security/audit-log.json` and `logging.js` defaulted `LOG_DIR` to `__dirname` (i.e. `/app/src/utils/`), so every error-log/audit-log write was landing in the image layer — a fresh container recreate would have wiped the entire audit log. Now both flow through `dataDir` which the start.sh bind mount already points at `/app/data`. Drive-by: removed unused `readline` import in `event-workers.js`. Also fixed a **test gap** in `__tests__/public-routes-drift.test.js`: `routes/security.js` was missing from the direct-mounts list, so the `/api/v1/security/events/ingest` and `/api/v1/security/events/batch` PUBLIC_ROUTES entries (added by DC-044) were flagged as stale. Added it with `/security` prefix mapping. **Pre-existing files on the running container (`audit-log.json` 319KB, `container-stats*.json` 186MB, `workflow-history.json` 269KB, `audit-log.json` etc.) are still in the image layer** — those are lost on next recreate unless a one-time migration step runs; out of scope for this fix but flagged for a follow-up. **Tests: 1214/1214 pass, +0 failures. ESLint: 146 warnings + 4 errors — identical to baseline (no new warnings/errors introduced).** Docker container does NOT need rebuilding: the affected code paths are evaluated at boot, and `dashcaddy-api/data/` is the existing bind mount — the new defaults resolve to the same path the container already uses via env vars (`CREDENTIALS_FILE=/app/data/credentials.json`, `ENCRYPTION_KEY_FILE=/app/data/.encryption-key`, etc.), and the env vars take precedence. Self-updater picks it up on the next release bump.
### DC-040: Investigate whether dashcaddy-post-deploy-patches.sh is still needed at all ### DC-040: Investigate whether dashcaddy-post-deploy-patches.sh is still needed at all
- **status:** todo - **status:** in-progress
- **owner:** unclaimed - **owner:** hermes
- **details:** The script applies 23+ `require()` path fixes on every update (audit from `BUILD-PIPELINE-FIX.md` shows it was created to paper over `dashcaddy-api/src/` being missing from tarballs). After the build-pipeline-fix (which now ships `src/` in every tarball), most of those patches should be no-ops. If any are still applying real changes, that means the source tree has a latent bug that DC-005-era refactors missed. Run `bash scripts/dashcaddy-post-deploy-patches.sh` against a fresh checkout of origin/main (or extract the v1.14.8 tarball to a clean dir) and count how many patches actually change anything vs are no-ops. If most are no-ops, the script can either be deleted entirely (cleanest) or kept as a defensive backstop with a comment explaining its purpose has shifted to "verify src/ shipped correctly." Effort: ~45 min. Risk: medium — safer to keep as backstop with reduced scope. - **details:** The script applies 23+ `require()` path fixes on every update (audit from `BUILD-PIPELINE-FIX.md` shows it was created to paper over `dashcaddy-api/src/` being missing from tarballs). After the build-pipeline-fix (which now ships `src/` in every tarball), most of those patches should be no-ops. If any are still applying real changes, that means the source tree has a latent bug that DC-005-era refactors missed. Run `bash scripts/dashcaddy-post-deploy-patches.sh` against a fresh checkout of origin/main (or extract the v1.14.8 tarball to a clean dir) and count how many patches actually change anything vs are no-ops. If most are no-ops, the script can either be deleted entirely (cleanest) or kept as a defensive backstop with a comment explaining its purpose has shifted to "verify src/ shipped correctly." Effort: ~45 min. Risk: medium — safer to keep as backstop with reduced scope.
- **impact:** Clarity. The current state — "script applies 23 fixes every update but only 3-4 actually do anything" — is opaque and brittle. - **impact:** Clarity. The current state — "script applies 23 fixes every update but only 3-4 actually do anything" — is opaque and brittle.