Fix Codacy audit findings: Docker, CI, deps, and dev tools#98
Merged
Conversation
Addresses Codacy findings for containers running as root. Add non-root appuser to alerter, collector, and server. Use built-in nginx user for client container. Add hadolint ignore for unpinned ca-certificates in server. Fixes part of #97 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Change binding from 0.0.0.0 to 127.0.0.1 since this is a local development tool. Fixes part of #97 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- urllib3 2.5.0 -> 2.6.3 (CVE-2026-21441, CVE-2025-66418, CVE-2025-66471) - requests 2.32.5 -> 2.33.1 (CVE-2026-25645) - Pygments 2.19.2 -> 2.20.0 (CVE-2026-4539) Fixes part of #97 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace mutable version tags with commit SHAs across all 9 workflow files. Version tags retained as trailing comments for readability. Fixes part of #97 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughPin GitHub Actions to commit SHAs across CI workflows; bump Go toolchain to 1.26.2 in CI and Dockerfiles; add non-root USERs in Dockerfiles; update Python deps in requirements.txt; restrict webhook listener to localhost; add async error handling in two React components; add Dependabot config. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 0 |
| Duplication | 0 |
TIP This summary will be updated as you push new changes. Give us feedback
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/release.yml (1)
39-39: Consider automating SHA refreshes for pinned actions.With this many pinned refs, add/confirm Dependabot updates for
github-actionsto reduce stale-SHA drift and manual maintenance.Based on learnings, this repository previously experienced breakage from bad SHA pinning, so automated updates/validation can reduce recurrence risk.
Also applies to: 241-241
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/release.yml at line 39, The workflow pins actions using a raw SHA (e.g., actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd); enable automated SHA refreshes by configuring Dependabot to update GitHub Actions: add/update a .github/dependabot.yml entry with package-ecosystem: "github-actions", target the repository (or paths containing your release workflow), set a sensible schedule, and allow the job to open PRs to refresh pinned refs (so actions like actions/checkout and any other pinned refs — including the one referenced at line ~241 — get updated automatically).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@server/Dockerfile`:
- Around line 41-42: The Dockerfile drops to a non-root user `appuser` but does
not ensure the declared VOLUME `/data` is owned/writable by that user; create
the directory and set ownership before switching users so `appuser` can write to
`/data`. Concretely, add a step (before `USER appuser`) that creates `/data` if
missing and chowns it to `appuser:appuser` (reference the `appuser` user and the
`/data` VOLUME declaration) so runtime writes to `/data` won’t fail after
dropping root.
---
Nitpick comments:
In @.github/workflows/release.yml:
- Line 39: The workflow pins actions using a raw SHA (e.g.,
actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd); enable automated SHA
refreshes by configuring Dependabot to update GitHub Actions: add/update a
.github/dependabot.yml entry with package-ecosystem: "github-actions", target
the repository (or paths containing your release workflow), set a sensible
schedule, and allow the job to open PRs to refresh pinned refs (so actions like
actions/checkout and any other pinned refs — including the one referenced at
line ~241 — get updated automatically).
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: bbf867ae-ca46-4ba2-a91d-0fa70928cbea
📒 Files selected for processing (15)
.github/workflows/ci-alerter.yml.github/workflows/ci-client.yml.github/workflows/ci-collector.yml.github/workflows/ci-docker.yml.github/workflows/ci-docs.yml.github/workflows/ci-server.yml.github/workflows/ci-walkthrough.yml.github/workflows/docker-publish.yml.github/workflows/release.ymlalerter/Dockerfileclient/Dockerfilecollector/Dockerfilerequirements.txtserver/Dockerfiletools/webhook_listener.py
10 tasks
Add error handling to promise chains in Chart data refresh and CopyCodeButton clipboard write to prevent unhandled rejection warnings. Fixes part of #97 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Update Go version across go.mod files, Dockerfiles, and CI workflows to fix stdlib CVEs in crypto/x509, html/template, archive/tar, and internal/syscall/unix. Fixes part of #97 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Keep Dockerfiles and CI at Go 1.26.2 for production builds, but allow local development with Go 1.26.1 by not forcing the toolchain directive. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (2)
client/src/components/Chart/Chart.tsx (1)
260-264: Consolidate refresh logic and handle refresh errors explicitly.Lines 263 and 287 suppress failures with empty catches, so refresh errors become invisible and debugging gets harder. Extract one
refreshDatacallback used by both paths, and log/report failures.As per coding guidelines: "`**/*.{go,js,ts,tsx}`: Minimize code duplication; refactor as needed."Proposed refactor
+ const refreshData = useCallback(() => { + if (!onDataRefresh) { + return; + } + onDataRefresh() + .then((refreshed) => { + setLiveData(refreshed); + }) + .catch((err) => { + console.error('Chart refresh failed:', err); + }); + }, [onDataRefresh]); + useEffect(() => { - if (!liveUpdate || !onDataRefresh) { + if (!liveUpdate) { return; } const id = setInterval(() => { - onDataRefresh().then((refreshed) => { - setLiveData(refreshed); - }).catch(() => {}); + refreshData(); }, updateInterval); return () => clearInterval(id); - }, [liveUpdate, onDataRefresh, updateInterval]); + }, [liveUpdate, refreshData, updateInterval]); @@ const handleRefresh = useCallback(() => { - if (onDataRefresh) { - onDataRefresh().then((refreshed) => { - setLiveData(refreshed); - }).catch(() => {}); - } - }, [onDataRefresh]); + refreshData(); + }, [refreshData]);Also applies to: 283-288
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/src/components/Chart/Chart.tsx` around lines 260 - 264, Extract a single async refreshData function (e.g., const refreshData = async () => { const refreshed = await onDataRefresh(); setLiveData(refreshed); }) and use it both inside the setInterval handler and wherever the second duplicate refresh call exists, replacing the duplicate .then/.catch blocks; ensure you add explicit error handling in refreshData (catch and call console.error or the app logger with the caught error) instead of swallowing errors with empty catches so refresh failures are visible.client/src/components/shared/CopyCodeButton.tsx (1)
29-34: Guard clipboard access and stop swallowing copy failures silently.Line 30 may throw synchronously when Clipboard API is unavailable, and Line 33 drops async failures with no signal. Please add an API guard and at least log/report the error (similar behavior exists in
client/src/components/AdminPanel/AdminTokenScopes.tsxLines 505-536).Proposed fix
- const handleCopy = useCallback(() => { - navigator.clipboard.writeText(code).then(() => { - setCopied(true); - setTimeout(() => setCopied(false), 2000); - }).catch(() => {}); - }, [code]); + const handleCopy = useCallback(async () => { + try { + if (!navigator.clipboard?.writeText) { + throw new Error('Clipboard API unavailable in this context.'); + } + await navigator.clipboard.writeText(code); + setCopied(true); + setTimeout(() => setCopied(false), 2000); + } catch (err) { + console.error('Failed to copy code:', err); + } + }, [code]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/src/components/shared/CopyCodeButton.tsx` around lines 29 - 34, handleCopy in CopyCodeButton currently calls navigator.clipboard.writeText without guarding for Clipboard API presence and swallows errors; update handleCopy to first check for navigator?.clipboard?.writeText and return a fallback if missing, then call writeText inside a try/catch (or await it) so failures are caught and reported (use console.error or the same reporting helper used in AdminTokenScopes) and only call setCopied(true) on success; ensure any async errors are logged instead of silently ignored.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@client/src/components/Chart/Chart.tsx`:
- Around line 260-264: Extract a single async refreshData function (e.g., const
refreshData = async () => { const refreshed = await onDataRefresh();
setLiveData(refreshed); }) and use it both inside the setInterval handler and
wherever the second duplicate refresh call exists, replacing the duplicate
.then/.catch blocks; ensure you add explicit error handling in refreshData
(catch and call console.error or the app logger with the caught error) instead
of swallowing errors with empty catches so refresh failures are visible.
In `@client/src/components/shared/CopyCodeButton.tsx`:
- Around line 29-34: handleCopy in CopyCodeButton currently calls
navigator.clipboard.writeText without guarding for Clipboard API presence and
swallows errors; update handleCopy to first check for
navigator?.clipboard?.writeText and return a fallback if missing, then call
writeText inside a try/catch (or await it) so failures are caught and reported
(use console.error or the same reporting helper used in AdminTokenScopes) and
only call setCopied(true) on success; ensure any async errors are logged instead
of silently ignored.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1b566294-ef89-4845-b3e7-3c1cf42feda8
📒 Files selected for processing (9)
.github/workflows/ci-alerter.yml.github/workflows/ci-collector.yml.github/workflows/ci-server.yml.github/workflows/release.ymlalerter/Dockerfileclient/src/components/Chart/Chart.tsxclient/src/components/shared/CopyCodeButton.tsxcollector/Dockerfileserver/Dockerfile
🚧 Files skipped from review as they are similar to previous changes (7)
- alerter/Dockerfile
- collector/Dockerfile
- .github/workflows/release.yml
- .github/workflows/ci-alerter.yml
- server/Dockerfile
- .github/workflows/ci-server.yml
- .github/workflows/ci-collector.yml
Ensure appuser owns /etc/pgedge (all services) and /data (server) so the non-root user can write to these directories at runtime. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Deduplicate Chart refresh logic into a shared callback and log errors. Guard clipboard API availability in CopyCodeButton and log failures instead of swallowing them silently. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Auto-update SHA-pinned action references weekly to prevent stale SHA drift. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
All 40 flagged locations use parameterized queries or trusted internal values. Adding nosemgrep: go-sql-concat-sqli to suppress confirmed false positives in Codacy scans. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This reverts commit 74bcf39.
This reverts commit dbe6af2.
…ndings" This reverts commit 8e291a8.
Add unknown type annotation to catch callback in Chart. Remove unnecessary clipboard API guard in CopyCodeButton. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The data directory is resolved from flags before the config file is loaded. Without --data-dir, it defaults to a path relative to the binary (/usr/local/bin/data) which the non-root appuser cannot create. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The server resolves its data directory from --data-dir flag before loading the config file. When invoked via docker compose exec (e.g., for user management), the CMD args don't apply and the binary falls back to a path relative to the executable. Create and chown this fallback directory. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Nginx must start as root to bind privileged port 80, then drops to the nginx user for worker processes via its own configuration. The USER directive prevented port binding. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
4 tasks
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
1 participant
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.
Summary
Addresses all 105 SRM security findings from the Codacy audit in #97.
40 findings are fixed by code/config changes; 58 are confirmed false
positives; 7 are exempt by project policy (MCP tools execute arbitrary
SQL by design).
Fixes
USERdirectivesto all 4 Dockerfiles. Add hadolint ignore for unpinned
ca-certificates.version tags with full 40-character commit SHAs across all 9
workflow files.
urllib32.5.0→2.6.3,
requests2.32.5→2.33.1,Pygments2.19.2→2.20.0to fix 5 known CVEs.
0.0.0.0to127.0.0.1.workflows to Go 1.26.2, fixing stdlib CVEs in crypto/x509,
html/template, archive/tar, and internal/syscall/unix. go.mod
toolchain stays at 1.26.1 for local dev compatibility.
rejections in Chart data refresh and CopyCodeButton clipboard
write.
Confirmed false positives (no code changes)
internal values. No user input reaches SQL without placeholders.
patterns.
secureCookieauto-detection.
Test plan
make test-allpasses (all Go and React tests, linting,coverage).
mkdocs build --strictworks with updated Python deps.Fixes #97
🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Security Improvements
Chores