fix: client dockerfile is missing hook package.json

[shikanime]

Issues liées

Issues numéro: #2353

---

Quel est le comportement actuel ?

Quel est le nouveau comportement ?

Cette PR introduit-elle un breaking change ?

Autres informations

[github-actions]

🤖 Hey !

The security scan report for the current pull request is available here.

[github-actions]

🤖 Hey !

The security scan report for the current pull request is available here.

[StephaneTrebel]

question: T'as testé avec pnpm run docker:ci ? Ou alors avec la Merge Queue ?
Je voudrais vraiment m'assurer qu'on n'aura aucun problème avec la target prod 😅

[github-actions]

🤖 Hey !

The security scan report for the current pull request is available here.

[shikanime]

Review: #2352 — fix: client dockerfile is missing hook package.json

Verdict: REQUEST CHANGES (build-breaking permission regression)

Findings

[blocker] apps/client/Dockerfile prod stage dropped ownership/permission setup. The base prod stage had:

COPY --chown=nginx:root --from=build /app/apps/client/dist /etc/nginx/html/
COPY --chown=nginx:root --chmod=660 ./apps/client/nginx/default.conf /etc/nginx/conf.d/default.conf
COPY --chown=nginx:root --chmod=750 ./apps/client/nginx/entrypoint.sh /docker-entrypoint.d/99-envsubst-js-assets.sh
RUN chmod -R g=u /etc/nginx/html

The PR head removes every --chown=nginx:root and the RUN chmod -R g=u /etc/nginx/html. nginxinc/nginx-unprivileged runs as USER 101 (nginx), so the now root:root 0660 default.conf and root:root html assets are not readable by nginx → nginx -t fails → container exits. Restore the --chown=nginx:root flags and the RUN chmod -R g=u /etc/nginx/html.

[warning] apps/server-nestjs/Dockerfile:93-94 — --chown=node:root dropped on the prod COPYs. App starts as USER node reading world-readable artifacts, but any runtime write under /app fails. Restore --chown=node:root.

[nit] Scope — the diff also bundles a 20-file pnpm-catalog migration unrelated to the bug. The hook fix itself (COPY packages/hooks/package.json before pnpm install) is correct and complete; consider splitting the catalog migration so the permission regression above is easier to catch.

Compliance checklist

• Stated bug fixed (hooks package.json copied before install in both Dockerfiles)
• No leaked secrets / no --network=host / --ignore-scripts retained
• Docker non-root best practice — regressed in client prod stage
• Image actually starts (nginx can't read root-owned 0660 config)

[github-actions]

🤖 Hey !

The security scan report for the current pull request is available here.

[github-actions]

🤖 Hey !

The security scan report for the current pull request is available here.

[github-actions]

🤖 Hey !

The security scan report for the current pull request is available here.

[shikanime]

Review — PR #2352 (fix: client dockerfile is missing hook package.json)

Verdict: REQUEST CHANGES — one blocker (runtime regression in client prod image), the rest is solid.

blocker

apps/client/Dockerfile — prod stage loses write permissions on /etc/nginx/html

The PR removed every --chown=nginx:root and the final RUN chmod -R g=u /etc/nginx/html from the prod stage (apps/client/Dockerfile:38-44).

Chain that breaks:

• dev/build stages run as root (no USER); COPY --from=build /app/apps/client/dist lands as root:root (PR dropped the --chown=nginx:root).
• prod is nginxinc/nginx-unprivileged, whose default USER is nginx (uid 101) — and the PR adds no USER root before the COPY. So COPY still runs as root and /etc/nginx/html is owned root:root, 755.
• At container start, /docker-entrypoint.d/99-envsubst-js-assets.sh (apps/client/nginx/entrypoint.sh:27-33) runs as nginx and does envsubst < file > file-out && mv file-out file inside /etc/nginx/html/assets — which it can no longer write to (owned root, mode 755).

On main this is masked by the dropped RUN chmod -R g=u /etc/nginx/html (root group = nginx's group in the unprivileged image, g=u makes it group-writable). Without it, the env-substitution step fails and the client serves un-substituted JS assets (or the entrypoint errors out).

Fix (restore the missing permission step):

COPY --from=build /app/apps/client/dist /etc/nginx/html/
COPY --chmod=660 ./apps/client/nginx/default.conf /etc/nginx/conf.d/default.conf
COPY --chmod=750 ./apps/client/nginx/entrypoint.sh /docker-entrypoint.d/99-envsubst-js-assets.sh
RUN chmod -R g=u /etc/nginx/html

(Keep the --chmod casts if you prefer them; the essential part is the chmod -R g=u after the COPY that lands as root.)

warnings (non-blocking, please confirm)

1. apps/server-nestjs/package.json:45,72 — silent major bumps via catalog. pnpm-workspace.yaml catalog defines @keycloak/keycloak-admin-client: ^26.5.5 and fastify-keycloak-adapter: ^3.0.0. On main, server-nestjs pinned @keycloak/keycloak-admin-client: ^24.0.0 and fastify-keycloak-adapter: 2.3.2. The catalog migration pulled both up two majors with no mention in the PR.

   • @keycloak/keycloak-admin-client v24→v26 is a real API bump and is actively imported (apps/server-nestjs/src/modules/keycloak/keycloak.module.ts:19, keycloak-client.service.ts). I built server-nestjs (passes) and ran the 394 unit tests (all pass), so the call sites still compile against v26 — but this should be an intentional, called-out upgrade, not a side-effect of de-duplication.
   • fastify-keycloak-adapter is not used anywhere in apps/server-nestjs (only the legacy apps/server). It appears to be a dead dependency that got re-cataloged. Consider dropping it from server-nestjs entirely.
     Recommendation: in pnpm-workspace.yaml, keep runtime at the versions the apps actually pin, and note the v24→v26 intent in the PR body / commit.

2. apps/client/Dockerfile:16 adds COPY packages/hooks/package.json but the client never depends on @cpn-console/hooks. apps/client/package.json only deps @cpn-console/logger and @cpn-console/shared. The pnpm install works without it (the PR's stated "missing hook package.json" rationale doesn't apply to the client build graph). Harmless but misleading — drop the line unless the client really needs hooks.

nits

• apps/server-nestjs/src/modules/infrastructure/configuration/configuration.service.ts:133-137 — getInternalOrPublicArgoCDUrl() logs the resolved URL on every call. Fine, just note it adds a log line per health-check tick.
• apps/server-nestjs/src/modules/registry/registry-http-client.service.ts:16,26,34 — status: HttpStatus (enum) is looser than number but compiles and is harmless; not worth changing.

Verified

• pnpm install --frozen-lockfile ✅ (catalog ↔ lockfile ↔ package.json consistent)
• apps/server-nestjs build (pnpm run build) ✅
• apps/server-nestjs unit tests: 394 passed, 50 skipped (e2e specs need a live DB) ✅
• ESLint on the changed source files ✅
• Could not run a live docker build — the local Docker daemon is down (containerd metadata I/O error). The blocker above is established by static analysis of the ownership/USER chain, not a failed build.

Release-managed files

.release-please-manifest.json / CHANGELOG.md / RELEASE.md are release-please/CI artifacts present in the branch history (commit b35fe1a1d0, 707c10cc58); they aren't authored by this PR's fix commits and shouldn't be merged as part of it — let release-please own them.

[github-actions]

🤖 Hey !

The security scan report for the current pull request is available here.

[github-actions]

🤖 Hey !

The security scan report for the current pull request is available here.

[github-actions]

🤖 Hey !

The security scan report for the current pull request is available here.

[cloud-pi-native-sonarqube]

Quality Gate passed

Issues

• 0 New Issues
• 0 Fixed Issues
• 0 Accepted Issues

Measures

• 0 Security Hotspots
• No data about coverage (26.20% Estimated after merge)
• 0.00% Duplicated Code (4.30% Estimated after merge)

Project ID: cloud-pi-native_console_AYsa46O31ebDtzs-pYyZ

View in SonarQube

[github-actions]

🤖 Hey !

The security scan report for the current pull request is available here.