Skip to content
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions .github/workflows/docker-publish.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
name: Docker Image CI

on:
push:
branches: [ "main" ]
pull_request:
branches: [ "main" ]

jobs:
build_and_push:
runs-on: ubuntu-latest
steps:
- name: Checkout code
uses: actions/checkout@v4

- name: Login to Docker Hub
uses: docker/login-action@v3
with:
username: ${{ secrets.DOCKERHUB_USERNAME }}
password: ${{ secrets.DOCKERHUB_TOKEN }}

- name: Build and push Docker image
uses: docker/build-push-action@v5
with:
context: .
push: true
tags: highkay/chatbox-web:latest
Comment on lines +16 to +27
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

⚠️ Potential issue

Do not push images or require secrets on PRs; gate login/push to main pushes only.

Right now:

  • Secrets are referenced on PRs, which won’t be available for forks and will fail the job.
  • The build step always pushes to Docker Hub, including for PRs.

Gate Docker Hub login and pushing to main branch pushes only, and build PRs without pushing.

Apply:

       - name: Login to Docker Hub
+        if: github.event_name == 'push' && github.ref == 'refs/heads/main'
         uses: docker/login-action@v3
         with:
           username: ${{ secrets.DOCKERHUB_USERNAME }}
           password: ${{ secrets.DOCKERHUB_TOKEN }}

-      - name: Build and push Docker image
+      - name: Build Docker image (no push on PRs)
         uses: docker/build-push-action@v5
         with:
           context: .
-          push: true
-          tags: highkay/chatbox-web:latest
+          push: ${{ github.event_name == 'push' && github.ref == 'refs/heads/main' }}
+          tags: |
+            highkay/chatbox-web:latest
+            highkay/chatbox-web:sha-${{ github.sha }}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- name: Login to Docker Hub
uses: docker/login-action@v3
with:
username: ${{ secrets.DOCKERHUB_USERNAME }}
password: ${{ secrets.DOCKERHUB_TOKEN }}
- name: Build and push Docker image
uses: docker/build-push-action@v5
with:
context: .
push: true
tags: highkay/chatbox-web:latest
- name: Login to Docker Hub
if: github.event_name == 'push' && github.ref == 'refs/heads/main'
uses: docker/login-action@v3
with:
username: ${{ secrets.DOCKERHUB_USERNAME }}
password: ${{ secrets.DOCKERHUB_TOKEN }}
- name: Build Docker image (no push on PRs)
uses: docker/build-push-action@v5
with:
context: .
push: ${{ github.event_name == 'push' && github.ref == 'refs/heads/main' }}
tags: |
highkay/chatbox-web:latest
highkay/chatbox-web:sha-${{ github.sha }}
🧰 Tools
🪛 YAMLlint (1.37.1)

[error] 27-27: no new line character at the end of file

(new-line-at-end-of-file)

🤖 Prompt for AI Agents
In .github/workflows/docker-publish.yml around lines 16–27, the workflow logs
into Docker Hub and pushes images unconditionally which references secrets on
PRs and pushes from forks; change the workflow so the docker/login-action step
and the build-and-push step that pushes are gated to only run on main branch
pushes (e.g. add an if: condition like github.ref == 'refs/heads/main' or
github.event_name == 'push' && github.ref == 'refs/heads/main' to those steps),
and keep a separate build-only step that runs for PRs without using secrets or
pushing (set push: false or run docker build without login). Ensure no step
references DOCKERHUB_USERNAME/DOCKERHUB_TOKEN unless behind the main-branch-only
if condition so secrets are never required on PRs.

21 changes: 21 additions & 0 deletions Dockerfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
# ---- Base ----
FROM node:22-alpine AS base
WORKDIR /app
COPY package.json package-lock.json* ./

# ---- Dependencies ----
FROM base AS dependencies
RUN npm config set registry https://registry.npmmirror.com/ && \
npm install --ignore-scripts && \
npm cache clean --force
Comment on lines +8 to +10
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Prefer reproducible installs and configurable registry; avoid hard-coding a third-party mirror.

  • Hard-coding npmmirror.com is a supply chain and availability risk.
  • Use npm ci for reproducible builds when a lockfile exists.
  • Keep the default registry unless overridden via build-arg.

Apply:

-FROM base AS dependencies
-RUN npm config set registry https://registry.npmmirror.com/ && \
-    npm install --ignore-scripts && \
-    npm cache clean --force
+FROM base AS dependencies
+# Allow overriding registry at build-time: docker build --build-arg NPM_REGISTRY=https://registry.npmmirror.com/ .
+ARG NPM_REGISTRY=https://registry.npmjs.org/
+RUN npm config set registry ${NPM_REGISTRY}
+# Use npm ci when lockfile exists; fallback to install otherwise
+RUN if [ -f package-lock.json ]; then \
+      npm ci --ignore-scripts; \
+    else \
+      npm install --ignore-scripts; \
+    fi && \
+    npm cache clean --force

Please confirm that build does not rely on postinstall scripts (ignore-scripts is enabled). If it does, we should drop the flag.


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== Locate Dockerfile(s) ==="
find . -type f -name Dockerfile -print -exec printf "\n--- %s ---\n\n" {} \; -exec sed -n '1,160p' {} \; || true

echo -e "\n=== Check for lockfiles at repo root ==="
for f in package-lock.json npm-shrinkwrap.json yarn.lock pnpm-lock.yaml; do
  if [ -f "$f" ]; then
    echo "FOUND: $f"
    ls -l "$f"
  fi
done

echo -e "\n=== Top-level package.json scripts (if exists) ==="
if [ -f package.json ]; then
  if command -v jq >/dev/null 2>&1; then
    jq '.scripts // {}' package.json || true
  else
    echo "(jq not available) showing package.json scripts via grep:"
    sed -n '1,240p' package.json | sed -n '/"scripts"/,/"dependencies"/p' || true
  fi
else
  echo "No top-level package.json found."
fi

echo -e "\n=== Search repository for any 'postinstall' scripts/mentions ==="
rg -n --hidden --no-ignore-vcs 'postinstall' || true

echo -e "\n=== Search for other package.json files (workspaces/monorepo) and their scripts (limited listing) ==="
rg -n --hidden --no-ignore-vcs '"scripts"\s*:\s*\{' --glob '!**/node_modules/**' || true
find . -name package.json -not -path "./node_modules/*" -print -exec sh -c 'printf "\n-- %s --\n" "$1"; if command -v jq >/dev/null 2>&1; then jq ".name, .scripts // {}" "$1"; else sed -n "1,240p" "$1" | rg -n "\"scripts\"|\"postinstall\"|\"name\"" || true; fi' sh {} \;

Length of output: 7390


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== Root Dockerfile ==="
if [ -f Dockerfile ]; then
  nl -ba Dockerfile | sed -n '1,240p'
else
  echo "No root Dockerfile"
fi

echo -e "\n=== team-sharing/Dockerfile ==="
if [ -f team-sharing/Dockerfile ]; then
  nl -ba team-sharing/Dockerfile | sed -n '1,240p'
else
  echo "No team-sharing/Dockerfile"
fi

echo -e "\n=== Search for hard-coded registries / npmmirror mentions ==="
rg -n --hidden --no-ignore-vcs 'npmmirror|registry.npmmirror.com|npm config set registry' --glob '!**/node_modules/**' || true

echo -e "\n=== Search for npm install / npm ci usages and --ignore-scripts occurrences ==="
rg -n --hidden --no-ignore-vcs 'npm (install|ci).*--ignore-scripts|npm ci|npm install' --glob '!**/node_modules/**' || true

echo -e "\n=== Show package.json postinstall scripts (all package.json files) ==="
find . -name package.json -not -path "./node_modules/*" -print -exec sh -c 'echo; echo "-- $1 --"; sed -n "1,240p" "$1" | sed -n "/\"postinstall\"/,/},/p"' sh {} \; || true

Length of output: 229


Fix required: don't hard-code a third‑party npm mirror; use npm ci and do not ignore postinstall scripts

Verified: repo contains package-lock.json and package.json defines important postinstall steps (root package.json and release/app/package.json; webpack config also invokes npm run postinstall). The Dockerfile snippet in the review hard‑codes registry to registry.npmmirror.com and uses --ignore-scripts, which will skip required postinstall hooks and likely break the build.

Files needing attention:

  • Dockerfile (repo root) — lines ~8–10 (replace hard-coded registry and remove --ignore-scripts)
  • team-sharing/Dockerfile — check for same pattern
  • package.json (root) and release/app/package.json — contain postinstall scripts (confirm behaviour)

Apply this patch (example):

-FROM base AS dependencies
-RUN npm config set registry https://registry.npmmirror.com/ && \
-    npm install --ignore-scripts && \
-    npm cache clean --force
+FROM base AS dependencies
+# Allow overriding registry at build-time: docker build --build-arg NPM_REGISTRY=...
+ARG NPM_REGISTRY=https://registry.npmjs.org/
+RUN npm config set registry ${NPM_REGISTRY}
+# Use npm ci when a lockfile exists for reproducible installs; DO NOT skip lifecycle scripts
+RUN if [ -f package-lock.json ]; then \
+      npm ci; \
+    else \
+      npm install; \
+    fi && \
+    npm cache clean --force

Action required: remove --ignore-scripts (build relies on postinstall). Also audit other Dockerfiles (team-sharing) and CI/build pipelines for hard-coded registries and update to use a build-arg or the default registry.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
RUN npm config set registry https://registry.npmmirror.com/ && \
npm install --ignore-scripts && \
npm cache clean --force
# Allow overriding registry at build-time: docker build --build-arg NPM_REGISTRY=...
ARG NPM_REGISTRY=https://registry.npmjs.org/
RUN npm config set registry ${NPM_REGISTRY}
# Use npm ci when a lockfile exists for reproducible installs; DO NOT skip lifecycle scripts
RUN if [ -f package-lock.json ]; then \
npm ci; \
else \
npm install; \
fi && \
npm cache clean --force
🤖 Prompt for AI Agents
In Dockerfile around lines 8 to 10, the build currently hard-codes
registry.npmmirror.com and uses "npm install --ignore-scripts", which skips
required postinstall hooks; replace this with "npm ci" (to use package-lock.json
and produce reproducible installs), remove the --ignore-scripts flag so
postinstall scripts run, and stop hard-coding the registry by accepting a build
ARG (defaulting to the public npm registry) or using the default registry; also
search team-sharing/Dockerfile and any CI/build pipeline steps for the same
pattern and apply the same changes there so postinstall hooks in root
package.json and release/app/package.json execute correctly.


# ---- Build ----
FROM dependencies AS build
COPY . .
RUN npm run build:web

# ---- Production ----
FROM nginx:alpine AS production
COPY --from=build /app/release/app/dist/renderer /usr/share/nginx/html
EXPOSE 80
CMD ["nginx", "-g", "daemon off;"]
9 changes: 9 additions & 0 deletions docker-compose.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
version: '3.8'

services:
chatbox:
build: .
ports:
- "3000:3000"
Comment on lines +6 to +7
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Port mismatch: container serves on 80, but compose maps to 3000:3000.

The Dockerfile exposes 80 (nginx). The current mapping publishes container port 3000 which won’t receive traffic.

Apply:

-    ports:
-      - "3000:3000"
+    ports:
+      - "3000:80"
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
ports:
- "3000:3000"
ports:
- "3000:80"
🤖 Prompt for AI Agents
In docker-compose.yml around lines 6 to 7, the compose mapping uses "3000:3000"
but the container (nginx) listens on port 80; update the ports mapping so the
host port maps to container port 80 (for example change to "3000:80" if you want
host 3000 to reach container 80, or "80:80" if you prefer host 80), and ensure
the Dockerfile still exposes port 80.

volumes:
- ./data:/root/.config/xyz.chatboxapp.ce
Comment on lines +5 to +9
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Align compose with published image and remove unused volume.

  • The nginx-based image won’t use /root/.config/xyz.chatboxapp.ce. This looks unrelated to a static web server.
  • Since CI pushes highkay/chatbox-web:latest, you can use that image directly in compose.

Apply:

   chatbox:
-    build: .
+    image: highkay/chatbox-web:latest
     ports:
       - "3000:80"
-    volumes:
-      - ./data:/root/.config/xyz.chatboxapp.ce
+    restart: unless-stopped
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
build: .
ports:
- "3000:3000"
volumes:
- ./data:/root/.config/xyz.chatboxapp.ce
chatbox:
image: highkay/chatbox-web:latest
ports:
- "3000:80"
restart: unless-stopped
🤖 Prompt for AI Agents
In docker-compose.yml around lines 5 to 9, the service is building locally and
mounting a volume ./data to /root/.config/xyz.chatboxapp.ce which is unused by
the nginx-based image; replace the build directive with image:
highkay/chatbox-web:latest and remove the volumes entry so the compose file uses
the published image and no longer mounts the irrelevant ./data volume.

Loading