Feat/ci cd - #37
Open
RayaneHassani wants to merge 10 commits into
Open
Conversation
- install.sh: bootstrap (Docker, ufw, fail2ban), generates .env with random secrets, builds the stack and waits for backend health - Caddyfile: reverse proxy with automatic HTTPS, domain from DOMAIN env - docker-compose.prod.yml: production overlay running Caddy on 80/443 - docker-compose.yml: bind app ports to 127.0.0.1 (only Caddy is public) - .gitignore: ignore per-instance config/ - .gitattributes: enforce LF line endings
- ask(): always return 0 (a non-empty answer with no default made the trailing test return non-zero, which set -e turned into an exit) - create .env only after all prompts succeed (no half-written file)
- update.sh: pull, rebuild, health-check; on failure rolls back code and restores the pre-update database dump. Flags: --yes, --no-backup. Single-instance lock; keeps the last 7 DB backups. - deploy/systemd: optional service + timer for scheduled local updates - .gitignore: ignore /backups/
- add micrometer-registry-prometheus; expose health, info, prometheus - enable liveness/readiness health probes - logback-spring.xml: console logs in dev, JSON logs in prod (Loki-ready) - permit /actuator/info and /actuator/prometheus (internal network only) - add JaCoCo plugin for coverage reports
- ci.yml: paths-filtered jobs on dev/main (push + PR), concurrency cancel - frontend: lint, typecheck, build - backend: compile, verify (test + JaCoCo), coverage artifact - governance: Flyway migration + i18n parity guards - secrets: gitleaks; dependency-review on PRs; docker compose build - ci-required: single aggregate status check for branch protection - scripts/ci: flyway-governance.sh, i18n-parity.sh (shared with GitLab later) - frontend: add typecheck script (tsc --noEmit) - third-party actions pinned by commit SHA - remove build-services.yml (superseded)
- CodeQL SAST for Java and JavaScript/TypeScript - Trivy filesystem and config scans, results to GitHub code scanning - OpenSSF Scorecard with published results - CycloneDX SBOM with build-provenance attestation - third-party actions pinned by commit SHA
- deploy.yml: SSH-based CD running update.sh on the server; prod (main) and staging (dev) targets, each gated by a repo variable so forks skip it - dependabot.yml: per-directory updates (maven, npm, docker, actions); groups minor/patch; ignores the Next.js fork - .gitlab-ci.yml: mirror reusing scripts/ci and native GitLab security templates - README: CI, Security, Scorecard and license badges
- backend job: add ephemeral Postgres service so @SpringBootTest can load - secrets job: run gitleaks binary directly (no org license required)
- notify job aggregates frontend, backend, governance, secrets, docker - no-ops when DISCORD_WEBHOOK_URL is unset (fork-safe); action pinned by SHA
Add docker-compose.monitoring.yml with Prometheus, Grafana, Loki, Promtail, node-exporter and cAdvisor, all bound to the internal network with no public port. Grafana is reachable via SSH tunnel only. Prometheus scrapes the backend, host, containers and Caddy; retention is 15d for metrics and 7d for logs. Grafana ships provisioned datasources and dashboards (JVM/Spring, VPS system, containers, logs). Expose Caddy's admin metrics endpoint on the internal network and add a `make monitoring` target plus README instructions.
📝 WalkthroughWalkthroughThe PR adds production installation and update automation, Caddy-based HTTPS routing, an optional Prometheus/Loki/Grafana monitoring stack, application telemetry configuration, and new GitHub/GitLab CI, security, dependency, and governance workflows. ChangesDeployment and runtime operations
Monitoring and application telemetry
CI and repository governance
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant Caddy
participant Frontend
participant Prometheus
participant Grafana
User->>Caddy: HTTPS request
Caddy->>Frontend: proxy request
Prometheus->>Frontend: scrape available metrics
Grafana->>Prometheus: query dashboard metrics
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Member
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (3)
.github/workflows/ci.yml (2)
149-155: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winGitleaks binary is downloaded and executed without checksum verification.
The release tarball is fetched over HTTPS and piped straight into
tar/execution with no SHA256 check against gitleaks' published checksums. Pinning the version helps, but doesn't protect against a compromised release asset.🔒 Add checksum verification
- name: Run gitleaks env: GITLEAKS_VERSION: 8.21.2 run: | + curl -sSfL "https://github.com/gitleaks/gitleaks/releases/download/v${GITLEAKS_VERSION}/gitleaks_${GITLEAKS_VERSION}_checksums.txt" -o checksums.txt curl -sSfL "https://github.com/gitleaks/gitleaks/releases/download/v${GITLEAKS_VERSION}/gitleaks_${GITLEAKS_VERSION}_linux_x64.tar.gz" \ - | tar -xz gitleaks + -o gitleaks.tar.gz + sha256sum --ignore-missing -c checksums.txt + tar -xzf gitleaks.tar.gz gitleaks ./gitleaks detect --source . --redact --verbose --exit-code 1🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/ci.yml around lines 149 - 155, Update the “Run gitleaks” workflow step to download the pinned release archive to a file, obtain the corresponding published SHA256 checksum, and verify the archive before extracting or executing it. Make the step fail on checksum mismatch, then preserve the existing gitleaks detect invocation and flags.
29-29: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winSet
persist-credentials: falseonactions/checkoutsteps that don't need to push. All of these checkouts leave the GITHUB_TOKEN persisted in.git/configfor the rest of the job (zizmorartipacked);security.yml'sscorecardjob (line 110-112) already does this correctly and can serve as the template for the rest.
.github/workflows/ci.yml#L29-L29: addwith: persist-credentials: falseto thechangesjob's checkout..github/workflows/ci.yml#L53-L53: add it to thefrontendjob's checkout (runspnpm install/build scripts)..github/workflows/ci.yml#L99-L99: add it to thebackendjob's checkout (runs Maven build)..github/workflows/ci.yml#L127-L129: add it to thegovernancejob's checkout..github/workflows/ci.yml#L144-L148: add it to thesecretsjob's checkout..github/workflows/ci.yml#L166-L166: add it to thedependency-reviewjob's checkout (this job also holdspull-requests: write, raising the stakes if leaked)..github/workflows/ci.yml#L183-L183: add it to thedockerjob's checkout..github/workflows/security.yml#L29-L29: add it to thecodeql-javajob's checkout..github/workflows/security.yml#L52-L52: add it to thecodeql-jsjob's checkout..github/workflows/security.yml#L69-L69: add it to thetrivyjob's checkout..github/workflows/security.yml#L133-L133: add it to thesbomjob's checkout (holdsid-token: write/attestations: write).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/ci.yml at line 29, Disable credential persistence for every listed actions/checkout step by adding persist-credentials: false under with: in .github/workflows/ci.yml lines 29, 53, 99, 127-129, 144-148, 166, and 183, and .github/workflows/security.yml lines 29, 52, 69, and 133; follow the existing scorecard checkout configuration in security.yml as the template.Source: Linters/SAST tools
monitoring/grafana/dashboards/containers.json (1)
25-27: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider using
container_memory_working_set_bytesinstead ofcontainer_memory_usage_bytes.
container_memory_usage_bytesincludes cached filesystem data (page cache), which can make memory usage appear inflated and artificially close to the limit.container_memory_working_set_bytesreflects the memory that cannot be evicted and is the metric the OOM killer watches.💡 Proposed change
"targets": [ - { "refId": "A", "expr": "sum by (name) (container_memory_usage_bytes{name=~\"codestar-.+\"})", "legendFormat": "{{name}}" } + { "refId": "A", "expr": "sum by (name) (container_memory_working_set_bytes{name=~\"codestar-.+\"})", "legendFormat": "{{name}}" } ]🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@monitoring/grafana/dashboards/containers.json` around lines 25 - 27, Update the Grafana dashboard query in the target identified by refId A to use container_memory_working_set_bytes instead of container_memory_usage_bytes, while preserving the existing name grouping, codestar-.+ filter, and legend format.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.gitlab-ci.yml:
- Around line 38-55: Add the Postgres service and matching DB_URL, DB_USER, and
DB_PASSWORD variables to the GitLab backend job, mirroring the existing
configuration in the GitHub CI workflow. Keep the service available before the
existing ./mvnw verify command runs.
- Around line 57-70: Update the governance job in .gitlab-ci.yml to set
GIT_DEPTH: 0, ensuring the checkout includes full history before the git diff
checks for migration and frontend message changes run.
In `@install.sh`:
- Line 166: Update the admin email summary output to use the populated
ADMIN_EMAIL variable instead of CODESTAR_BOOTSTRAP_SUPER_ADMIN_EMAIL, so the
address entered during set_env is displayed rather than the fallback text.
- Around line 107-111: Update the UFW setup around the existing firewall
commands to detect and allow the system’s active SSH port for TCP instead of
hardcoding 22/tcp, preserving the current administrator connection before
enabling UFW. Also add an allow rule for 443/udp alongside the existing 443/tcp
rule so HTTP/3 traffic remains available.
- Line 80: Update the Docker health-status lookup in the install script to
invoke `docker inspect` through the existing `$SUDO` prefix, preserving the
current error suppression and `missing` fallback. Ensure the command works for
non-root execution paths that rely on `$SUDO` for Docker daemon access.
In `@monitoring/grafana/dashboards/jvm-spring.json`:
- Around line 45-47: Enable the Spring HTTP request histogram setting by adding
management.metrics.distribution.percentiles-histogram.http.server.requests=true
to the backend configuration used by this dashboard, so the existing p95 query
for http_server_requests_seconds_bucket receives data.
In `@update.sh`:
- Around line 38-41: Update the logging helpers log, ok, warn, and err to write
all diagnostic output to stderr, keeping stdout clean for command substitutions
such as backup_db. Preserve their existing formatting, colors, and messages
while applying the stderr redirection consistently.
- Around line 44-45: Replace the hardcoded /tmp/codestar-update.lock target in
the update script’s flock setup with a lockfile located in the repository
directory, while preserving the existing nonblocking flock and “Another update
is already running.” failure behavior.
- Around line 132-134: Update the rollback sequence around restore_db and
$COMPOSE so the backend is explicitly stopped before any database restore, then
restore the backup with a real conditional that distinguishes a missing
BACKUP_FILE from a failed restore_db call. Ensure restore_db failures remain
visible and do not emit the “backup was skipped” warning; only log that warning
when no backup exists, and start the backend again after restoration.
---
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 149-155: Update the “Run gitleaks” workflow step to download the
pinned release archive to a file, obtain the corresponding published SHA256
checksum, and verify the archive before extracting or executing it. Make the
step fail on checksum mismatch, then preserve the existing gitleaks detect
invocation and flags.
- Line 29: Disable credential persistence for every listed actions/checkout step
by adding persist-credentials: false under with: in .github/workflows/ci.yml
lines 29, 53, 99, 127-129, 144-148, 166, and 183, and
.github/workflows/security.yml lines 29, 52, 69, and 133; follow the existing
scorecard checkout configuration in security.yml as the template.
In `@monitoring/grafana/dashboards/containers.json`:
- Around line 25-27: Update the Grafana dashboard query in the target identified
by refId A to use container_memory_working_set_bytes instead of
container_memory_usage_bytes, while preserving the existing name grouping,
codestar-.+ filter, and legend format.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 7f6ac550-cf4f-4abf-bf9a-9d4ac510ab0a
📒 Files selected for processing (35)
.env.example.gitattributes.github/dependabot.yml.github/workflows/build-services.yml.github/workflows/ci.yml.github/workflows/deploy.yml.github/workflows/security.yml.gitignore.gitlab-ci.ymlCaddyfileMakefileREADME.mdapps/backend/pom.xmlapps/backend/src/main/java/com/codestar/backend/config/SecurityConfig.javaapps/backend/src/main/resources/application.propertiesapps/backend/src/main/resources/logback-spring.xmlapps/frontend/package.jsondeploy/systemd/codestar-update.servicedeploy/systemd/codestar-update.timerdocker-compose.monitoring.ymldocker-compose.prod.ymldocker-compose.ymlinstall.shmonitoring/grafana/dashboards/containers.jsonmonitoring/grafana/dashboards/jvm-spring.jsonmonitoring/grafana/dashboards/logs.jsonmonitoring/grafana/dashboards/vps-system.jsonmonitoring/grafana/provisioning/dashboards/dashboards.ymlmonitoring/grafana/provisioning/datasources/datasources.ymlmonitoring/loki/loki-config.ymlmonitoring/prometheus/prometheus.ymlmonitoring/promtail/promtail-config.ymlscripts/ci/flyway-governance.shscripts/ci/i18n-parity.shupdate.sh
💤 Files with no reviewable changes (1)
- .github/workflows/build-services.yml
Comment on lines
+38
to
+55
| backend: | ||
| stage: build | ||
| image: maven:3.9-eclipse-temurin-17 | ||
| rules: | ||
| - changes: [apps/backend/**/*] | ||
| cache: | ||
| key: maven | ||
| paths: [.m2/repository] | ||
| variables: | ||
| MAVEN_OPTS: "-Dmaven.repo.local=$CI_PROJECT_DIR/.m2/repository" | ||
| script: | ||
| - cd apps/backend | ||
| - ./mvnw verify -B --no-transfer-progress | ||
| artifacts: | ||
| when: always | ||
| paths: [apps/backend/target/site/jacoco/] | ||
| expire_in: 7 days | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== Files ==\n'
git ls-files .gitlab-ci.yml .github/workflows/ci.yml apps/backend/**/application*.yml apps/backend/**/application*.yaml apps/backend/**/application*.properties 2>/dev/null || true
printf '\n== .gitlab-ci.yml (relevant lines) ==\n'
nl -ba .gitlab-ci.yml | sed -n '1,220p'
printf '\n== .github/workflows/ci.yml (relevant lines) ==\n'
nl -ba .github/workflows/ci.yml | sed -n '1,220p'
printf '\n== Backend config references to DB_* / postgres / flyway ==\n'
rg -n --hidden --glob 'apps/backend/**' 'DB_URL|DB_USER|DB_PASSWORD|postgres|flyway|datasource|jdbc:postgresql' apps/backend || true
printf '\n== Governance diff / fetch settings references ==\n'
rg -n --hidden --glob '.gitlab-ci.yml' 'GIT_DEPTH|fetch|diff|merge base|origin/\$CI_DEFAULT_BRANCH' .gitlab-ci.yml || trueRepository: CodeStar-Project/codestar
Length of output: 469
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== .gitlab-ci.yml ==\n'
sed -n '1,220p' .gitlab-ci.yml
printf '\n== .github/workflows/ci.yml ==\n'
sed -n '1,240p' .github/workflows/ci.yml
printf '\n== backend application properties ==\n'
sed -n '1,220p' apps/backend/src/main/resources/application.properties
printf '\n--- dev ---\n'
sed -n '1,220p' apps/backend/src/main/resources/application-dev.properties
printf '\n--- prod ---\n'
sed -n '1,220p' apps/backend/src/main/resources/application-prod.properties
printf '\n== DB/Flyway search ==\n'
rg -n --hidden 'DB_URL|DB_USER|DB_PASSWORD|spring\.datasource|flyway|jdbc:postgresql|postgres' apps/backend/src/main/resources apps/backend || trueRepository: CodeStar-Project/codestar
Length of output: 15293
Add Postgres to the GitLab backend job
apps/backend/src/main/resources/application.properties expects DB_URL, DB_USER, and DB_PASSWORD, so .gitlab-ci.yml needs the same Postgres service/env block as .github/workflows/ci.yml; otherwise ./mvnw verify has no database to run against and the job will fail.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.gitlab-ci.yml around lines 38 - 55, Add the Postgres service and matching
DB_URL, DB_USER, and DB_PASSWORD variables to the GitLab backend job, mirroring
the existing configuration in the GitHub CI workflow. Keep the service available
before the existing ./mvnw verify command runs.
Comment on lines
+57
to
+70
| governance: | ||
| stage: test | ||
| image: alpine:3.20 | ||
| before_script: | ||
| - apk add --no-cache bash git jq | ||
| script: | ||
| - | | ||
| if git diff --name-only "origin/$CI_DEFAULT_BRANCH"...HEAD -- apps/backend/src/main/resources/db/migration | grep -q .; then | ||
| BASE_REF="origin/$CI_DEFAULT_BRANCH" bash scripts/ci/flyway-governance.sh | ||
| fi | ||
| - | | ||
| if git diff --name-only "origin/$CI_DEFAULT_BRANCH"...HEAD -- apps/frontend/messages | grep -q .; then | ||
| bash scripts/ci/i18n-parity.sh | ||
| fi |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## .gitlab-ci.yml (relevant section)\n'
sed -n '1,140p' .gitlab-ci.yml
printf '\n## Search for depth/fetch settings and governance jobs\n'
rg -n "GIT_DEPTH|fetch-depth|governance:|git diff --name-only|origin/\\$CI_DEFAULT_BRANCH" .gitlab-ci.yml ci.yml .gitlab-ci.yml . 2>/dev/null || trueRepository: CodeStar-Project/codestar
Length of output: 2433
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## Search for depth settings and related CI files\n'
git ls-files | rg '(^|/)(\.gitlab-ci\.yml|ci\.yml|workflow|pipeline|actions|github/workflows/ci\.yml)$|(^|/)(gitlab-ci|ci)\.ya?ml$' || true
printf '\n## Search for depth/fetch references (literal pattern-safe)\n'
rg -n 'GIT_DEPTH|fetch-depth|fetch strategy|fetch-depth: 0|fetch_strategy|depth:' . --glob '.gitlab-ci.yml' --glob 'ci.yml' --glob '.github/workflows/ci.yml' || true
printf '\n## Search for the governance job in any CI file\n'
rg -n '^governance:|git diff --name-only "origin/\$CI_DEFAULT_BRANCH"\.\.\.HEAD' . --glob '.gitlab-ci.yml' --glob 'ci.yml' --glob '.github/workflows/ci.yml' || true
printf '\n## Probe git diff behavior in a shallow repo with no merge base\n'
tmp="$(mktemp -d)"
cd "$tmp"
git init -q
git config user.email a@b.c
git config user.name test
echo one > f
git add f
git commit -q -m c1
git checkout -q -b feature
echo two > f
git commit -q -am c2
git checkout -q main
git branch -M main
git rev-parse HEAD >/dev/null
cd "$tmp/.."
git clone -q --depth 1 "file://$tmp" shallow
cd shallow
set +e
out=$(git diff --name-only origin/main...HEAD 2>&1)
status=$?
set -e
printf 'status=%s\n%s\n' "$status" "$out"Repository: CodeStar-Project/codestar
Length of output: 865
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## Locate the referenced CI config\n'
git ls-files | rg '(^|/)(ci\.yml|ci\.yaml|\.github/workflows/ci\.yml)$' || true
printf '\n## Read any matching CI config that exists\n'
for f in .github/workflows/ci.yml ci.yml; do
if [ -f "$f" ]; then
printf '\n### %s\n' "$f"
sed -n '1,220p' "$f"
fi
done
printf '\n## Search for any clone-depth override in tracked CI files\n'
rg -n 'GIT_DEPTH|fetch-depth|clone_depth|fetch strategy|fetch_strategy' . --glob '.gitlab-ci.yml' --glob 'ci.yml' --glob '.github/workflows/ci.yml' || true
printf '\n## Shallow-clone probe: diff against a branch without merge base\n'
tmpdir="$(mktemp -d)"
repo="$tmpdir/repo"
clone="$tmpdir/clone"
mkdir -p "$repo"
cd "$repo"
git init -q
git config user.email a@b.c
git config user.name test
printf 'a\n' > file.txt
git add file.txt
git commit -q -m "base"
git branch -M main
git checkout -q -b feature
printf 'b\n' > file.txt
git commit -q -am "feature"
git checkout -q main
printf 'c\n' > other.txt
git add other.txt
git commit -q -m "main update"
# create a shallow clone of feature only, then fetch main shallowly
cd "$tmpdir"
git clone -q --depth 1 --branch feature "file://$repo" "$clone"
cd "$clone"
git remote add origin "file://$repo"
git fetch -q --depth 1 origin main:refs/remotes/origin/main
set +e
out=$(git diff --name-only origin/main...HEAD 2>&1)
status=$?
set -e
printf 'status=%s\n%s\n' "$status" "$out"Repository: CodeStar-Project/codestar
Length of output: 7865
🏁 Script executed:
#!/bin/bash
set -euo pipefail
tmpdir="$(mktemp -d)"
repo="$tmpdir/repo.git"
work="$tmpdir/work"
clone="$tmpdir/clone"
git init -q --bare "$repo"
git clone -q "$repo" "$work"
cd "$work"
git config user.email a@b.c
git config user.name test
# main history
printf 'base\n' > file.txt
git add file.txt
git commit -q -m base
git branch -M main
git push -q -u origin main
printf 'main1\n' >> file.txt
git commit -q -am main1
git push -q
# feature branch from the first commit
base_commit="$(git rev-list --max-parents=0 HEAD)"
git checkout -q -b feature "$base_commit"
printf 'feature1\n' > feature.txt
git add feature.txt
git commit -q -m feature1
git push -q -u origin feature
# shallow clone feature only, then shallow-fetch main
cd "$tmpdir"
git clone -q --depth 1 --branch feature "file://$repo" "$clone"
cd "$clone"
git fetch -q --depth 1 origin main:refs/remotes/origin/main
printf 'refs:\n'
git show-ref --heads --tags --dereference | sed -n '1,10p'
printf '\nmerge-base:\n'
set +e
mb=$(git merge-base origin/main HEAD 2>&1)
mb_status=$?
printf 'status=%s\n%s\n' "$mb_status" "$mb"
printf '\ndiff:\n'
out=$(git diff --name-only origin/main...HEAD 2>&1)
diff_status=$?
printf 'status=%s\n%s\n' "$diff_status" "$out"Repository: CodeStar-Project/codestar
Length of output: 467
Ensure full history is available for the governance job
git diff origin/$CI_DEFAULT_BRANCH...HEAD needs the merge base, so this job should set GIT_DEPTH: 0 like the GitHub workflow; otherwise a shallow checkout can make the guards fail with “no merge base.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.gitlab-ci.yml around lines 57 - 70, Update the governance job in
.gitlab-ci.yml to set GIT_DEPTH: 0, ensuring the checkout includes full history
before the git diff checks for migration and frontend message changes run.
| wait_healthy() { | ||
| local name="$1" tries="${2:-60}" status | ||
| for _ in $(seq 1 "$tries"); do | ||
| status="$(docker inspect -f '{{.State.Health.Status}}' "$name" 2>/dev/null || echo missing)" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Use $SUDO for Docker daemon interactions.
If the script is executed by a non-root user via sudo (or relying on the script's internal $SUDO prefixing), docker inspect will fail with "permission denied" because it requires daemon access and is missing the $SUDO prefix. The error gets swallowed by 2>/dev/null, causing the loop to always return missing and fail the health check timeout unnecessarily.
🛠️ Proposed fix to prefix with `$SUDO`
- status="$(docker inspect -f '{{.State.Health.Status}}' "$name" 2>/dev/null || echo missing)"
+ status="$($SUDO docker inspect -f '{{.State.Health.Status}}' "$name" 2>/dev/null || echo missing)"📝 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
| status="$(docker inspect -f '{{.State.Health.Status}}' "$name" 2>/dev/null || echo missing)" | |
| status="$($SUDO docker inspect -f '{{.State.Health.Status}}' "$name" 2>/dev/null || echo missing)" |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@install.sh` at line 80, Update the Docker health-status lookup in the install
script to invoke `docker inspect` through the existing `$SUDO` prefix,
preserving the current error suppression and `missing` fallback. Ensure the
command works for non-root execution paths that rely on `$SUDO` for Docker
daemon access.
Comment on lines
+107
to
+111
| $SUDO ufw allow 22/tcp >/dev/null 2>&1 || true | ||
| $SUDO ufw allow 80/tcp >/dev/null 2>&1 || true | ||
| $SUDO ufw allow 443/tcp >/dev/null 2>&1 || true | ||
| $SUDO ufw --force enable >/dev/null 2>&1 || true | ||
| $SUDO systemctl enable --now fail2ban >/dev/null 2>&1 || true |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
Prevent SSH lockout and allow HTTP/3 (QUIC) traffic.
Hardcoding 22/tcp and forcefully enabling ufw will instantly sever the administrator's connection and lock them out if the server uses a non-standard SSH port.
Additionally, docker-compose.prod.yml rightfully binds 443/udp for Caddy to serve HTTP/3. However, ufw is only configured to allow TCP on 443, which will block QUIC traffic and cause connection delays as clients are forced to time out and fall back to TCP.
🔒️ Proposed fix to dynamically allow the active SSH port and allow UDP 443
- $SUDO ufw allow 22/tcp >/dev/null 2>&1 || true
+ # Dynamically determine the active SSH port to prevent lockouts
+ SSH_PORTS=$($SUDO sshd -T 2>/dev/null | awk 'tolower($1)=="port" {print $2}' || echo 22)
+ for p in ${SSH_PORTS:-22}; do
+ $SUDO ufw allow "$p"/tcp >/dev/null 2>&1 || true
+ done
$SUDO ufw allow 80/tcp >/dev/null 2>&1 || true
$SUDO ufw allow 443/tcp >/dev/null 2>&1 || true
+ $SUDO ufw allow 443/udp >/dev/null 2>&1 || true
$SUDO ufw --force enable >/dev/null 2>&1 || true📝 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
| $SUDO ufw allow 22/tcp >/dev/null 2>&1 || true | |
| $SUDO ufw allow 80/tcp >/dev/null 2>&1 || true | |
| $SUDO ufw allow 443/tcp >/dev/null 2>&1 || true | |
| $SUDO ufw --force enable >/dev/null 2>&1 || true | |
| $SUDO systemctl enable --now fail2ban >/dev/null 2>&1 || true | |
| # Dynamically determine the active SSH port to prevent lockouts | |
| SSH_PORTS=$($SUDO sshd -T 2>/dev/null | awk 'tolower($1)=="port" {print $2}' || echo 22) | |
| for p in ${SSH_PORTS:-22}; do | |
| $SUDO ufw allow "$p"/tcp >/dev/null 2>&1 || true | |
| done | |
| $SUDO ufw allow 80/tcp >/dev/null 2>&1 || true | |
| $SUDO ufw allow 443/tcp >/dev/null 2>&1 || true | |
| $SUDO ufw allow 443/udp >/dev/null 2>&1 || true | |
| $SUDO ufw --force enable >/dev/null 2>&1 || true | |
| $SUDO systemctl enable --now fail2ban >/dev/null 2>&1 || true |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@install.sh` around lines 107 - 111, Update the UFW setup around the existing
firewall commands to detect and allow the system’s active SSH port for TCP
instead of hardcoding 22/tcp, preserving the current administrator connection
before enabling UFW. Also add an allow rule for 443/udp alongside the existing
443/tcp rule so HTTP/3 traffic remains available.
| ${c_green}────────────────────────────────────────────────${c_reset} | ||
| ${c_green} Codestar is deploying.${c_reset} | ||
| URL: https://${DOMAIN} | ||
| Admin: ${CODESTAR_BOOTSTRAP_SUPER_ADMIN_EMAIL:-<from .env>} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Fix undefined variable in summary output.
The script uses set_env to write the admin email to the .env file, but CODESTAR_BOOTSTRAP_SUPER_ADMIN_EMAIL is never assigned as a variable within the script's own execution environment. Consequently, this will silently default to the <from .env> fallback string. Use the populated ADMIN_EMAIL variable instead to ensure the entered address is displayed back to the user correctly.
🐛 Proposed fix
- Admin: ${CODESTAR_BOOTSTRAP_SUPER_ADMIN_EMAIL:-<from .env>}
+ Admin: ${ADMIN_EMAIL:-<from .env>}📝 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
| Admin: ${CODESTAR_BOOTSTRAP_SUPER_ADMIN_EMAIL:-<from .env>} | |
| Admin: ${ADMIN_EMAIL:-<from .env>} |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@install.sh` at line 166, Update the admin email summary output to use the
populated ADMIN_EMAIL variable instead of CODESTAR_BOOTSTRAP_SUPER_ADMIN_EMAIL,
so the address entered during set_env is displayed rather than the fallback
text.
Comment on lines
+45
to
+47
| "targets": [ | ||
| { "refId": "A", "expr": "histogram_quantile(0.95, sum by (le) (rate(http_server_requests_seconds_bucket[5m])))", "legendFormat": "p95" } | ||
| ] |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if the histogram property is enabled for http.server.requests
rg 'management\.metrics\.distribution\.percentiles-histogram\.http\.server\.requests\s*=\s*true' apps/backend/src/main/resources/application.properties || echo "Property not found. You may need to add it to application.properties."Repository: CodeStar-Project/codestar
Length of output: 233
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '## percentiles-histogram matches\n'
rg -n --hidden --glob '!**/node_modules/**' --glob '!**/dist/**' --glob '!**/build/**' 'management\.metrics\.distribution\.percentiles-histogram|http_server_requests_seconds_bucket|percentiles-histogram' .
printf '\n## dashboard snippet\n'
sed -n '1,140p' monitoring/grafana/dashboards/jvm-spring.jsonRepository: CodeStar-Project/codestar
Length of output: 3421
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '## spring metrics config matches\n'
rg -n --hidden --glob '!**/node_modules/**' --glob '!**/dist/**' --glob '!**/build/**' \
'management\.metrics\.distribution\.percentiles-histogram|management\.metrics\.distribution|http\.server\.requests|meterFilter|MeterFilter|percentilesHistogram' \
. --glob '!*monitoring/grafana/dashboards/jvm-spring.json'
printf '\n## candidate config files\n'
git ls-files | rg 'application\.(properties|yml|yaml)$|bootstrap\.(properties|yml|yaml)$|.*\.properties$|.*\.ya?ml$'Repository: CodeStar-Project/codestar
Length of output: 196
Enable HTTP request histograms for the p95 panel. The dashboard query uses http_server_requests_seconds_bucket, but no backend Spring config enables management.metrics.distribution.percentiles-histogram.http.server.requests=true, so this panel will stay empty until that property is added.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@monitoring/grafana/dashboards/jvm-spring.json` around lines 45 - 47, Enable
the Spring HTTP request histogram setting by adding
management.metrics.distribution.percentiles-histogram.http.server.requests=true
to the backend configuration used by this dashboard, so the existing p95 query
for http_server_requests_seconds_bucket receives data.
Comment on lines
+38
to
+41
| log() { printf '%s==>%s %s\n' "$c_blue" "$c_reset" "$*"; } | ||
| ok() { printf '%s✓%s %s\n' "$c_green" "$c_reset" "$*"; } | ||
| warn() { printf '%s!%s %s\n' "$c_yellow" "$c_reset" "$*"; } | ||
| err() { printf '%s✗%s %s\n' "$c_red" "$c_reset" "$*" >&2; } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Direct logs to stderr to prevent corrupting command substitutions.
Currently, log, ok, and warn print to stdout. When backup_db is called via command substitution (BACKUP_FILE="$(backup_db)"), BACKUP_FILE captures these log messages along with the backup file path. This multi-line string containing ANSI escape codes will fail the [ -f "$f" ] check in restore_db, causing the database rollback to silently abort.
Direct all diagnostic and logging output to stderr so that stdout remains clean for returning data.
🐛 Proposed fix
-log() { printf '%s==>%s %s\n' "$c_blue" "$c_reset" "$*"; }
-ok() { printf '%s✓%s %s\n' "$c_green" "$c_reset" "$*"; }
-warn() { printf '%s!%s %s\n' "$c_yellow" "$c_reset" "$*"; }
+log() { printf '%s==>%s %s\n' "$c_blue" "$c_reset" "$*" >&2; }
+ok() { printf '%s✓%s %s\n' "$c_green" "$c_reset" "$*" >&2; }
+warn() { printf '%s!%s %s\n' "$c_yellow" "$c_reset" "$*" >&2; }
err() { printf '%s✗%s %s\n' "$c_red" "$c_reset" "$*" >&2; }📝 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
| log() { printf '%s==>%s %s\n' "$c_blue" "$c_reset" "$*"; } | |
| ok() { printf '%s✓%s %s\n' "$c_green" "$c_reset" "$*"; } | |
| warn() { printf '%s!%s %s\n' "$c_yellow" "$c_reset" "$*"; } | |
| err() { printf '%s✗%s %s\n' "$c_red" "$c_reset" "$*" >&2; } | |
| log() { printf '%s==>%s %s\n' "$c_blue" "$c_reset" "$*" >&2; } | |
| ok() { printf '%s✓%s %s\n' "$c_green" "$c_reset" "$*" >&2; } | |
| warn() { printf '%s!%s %s\n' "$c_yellow" "$c_reset" "$*" >&2; } | |
| err() { printf '%s✗%s %s\n' "$c_red" "$c_reset" "$*" >&2; } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@update.sh` around lines 38 - 41, Update the logging helpers log, ok, warn,
and err to write all diagnostic output to stderr, keeping stdout clean for
command substitutions such as backup_db. Preserve their existing formatting,
colors, and messages while applying the stderr redirection consistently.
Comment on lines
+44
to
+45
| exec 9>/tmp/codestar-update.lock | ||
| if ! flock -n 9; then err "Another update is already running."; exit 1; fi |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Avoid predictable lockfiles in /tmp/.
Writing a hardcoded lockfile to /tmp/codestar-update.lock is vulnerable to a local denial-of-service (DoS) or Symlink/TOCTOU attack. A malicious user on the system could pre-create the file or change its permissions, permanently blocking application updates.
Since this script is executed from the repository directory, use a local lockfile within the project directory.
🔒️ Proposed fix
-exec 9>/tmp/codestar-update.lock
+exec 9>./.update.lock
if ! flock -n 9; then err "Another update is already running."; exit 1; fi📝 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
| exec 9>/tmp/codestar-update.lock | |
| if ! flock -n 9; then err "Another update is already running."; exit 1; fi | |
| exec 9>./.update.lock | |
| if ! flock -n 9; then err "Another update is already running."; exit 1; fi |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@update.sh` around lines 44 - 45, Replace the hardcoded
/tmp/codestar-update.lock target in the update script’s flock setup with a
lockfile located in the repository directory, while preserving the existing
nonblocking flock and “Another update is already running.” failure behavior.
Source: Linters/SAST tools
Comment on lines
+132
to
+134
| git reset --hard "$OLD_COMMIT" | ||
| $COMPOSE up --build -d | ||
| [ -n "$BACKUP_FILE" ] && restore_db "$BACKUP_FILE" || warn "No DB restore (backup was skipped)." |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Prevent DB restore race conditions and fix unsafe conditional logic.
There are two critical issues in the rollback sequence:
- Race condition: Restoring the database after (or while) bringing the backend up allows the backend (which runs Flyway migrations on startup) to race against the
psqlrestore process. This can lead to deadlocks, locked relations, or corrupted schemas. The backend should be explicitly stopped before restoring the database. - Unsafe conditional: The
A && B || Cshorthand is not a trueif-then-else. Ifrestore_dbfails (returns a non-zero exit code), the||branch executes, misleadingly logging that the backup was skipped instead of surfacing the failure.
🐛 Proposed fix
git reset --hard "$OLD_COMMIT"
-$COMPOSE up --build -d
-[ -n "$BACKUP_FILE" ] && restore_db "$BACKUP_FILE" || warn "No DB restore (backup was skipped)."
+$COMPOSE stop backend
+if [ -n "$BACKUP_FILE" ]; then
+ restore_db "$BACKUP_FILE" || warn "Database restore failed!"
+else
+ warn "No DB restore (backup was skipped)."
+fi
+$COMPOSE up --build -d📝 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
| git reset --hard "$OLD_COMMIT" | |
| $COMPOSE up --build -d | |
| [ -n "$BACKUP_FILE" ] && restore_db "$BACKUP_FILE" || warn "No DB restore (backup was skipped)." | |
| git reset --hard "$OLD_COMMIT" | |
| $COMPOSE stop backend | |
| if [ -n "$BACKUP_FILE" ]; then | |
| restore_db "$BACKUP_FILE" || warn "Database restore failed!" | |
| else | |
| warn "No DB restore (backup was skipped)." | |
| fi | |
| $COMPOSE up --build -d |
🧰 Tools
🪛 Shellcheck (0.11.0)
[info] 134-134: Note that A && B || C is not if-then-else. C may run when A is true.
(SC2015)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@update.sh` around lines 132 - 134, Update the rollback sequence around
restore_db and $COMPOSE so the backend is explicitly stopped before any database
restore, then restore the backup with a real conditional that distinguishes a
missing BACKUP_FILE from a failed restore_db call. Ensure restore_db failures
remain visible and do not emit the “backup was skipped” warning; only log that
warning when no backup exists, and start the backend again after restoration.
Source: Linters/SAST tools
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR delivers the DevOps foundation for Codestar:
No frontend changes except a typecheck script in package.json used by CI.
Replaces #35 — see closing comment there for context.
Summary by CodeRabbit