Skip to content

fix(squid): run Squid container as non-root proxy user#1271

Merged
Mossaka merged 2 commits intomainfrom
fix/095-squid-non-root
Mar 13, 2026
Merged

fix(squid): run Squid container as non-root proxy user#1271
Mossaka merged 2 commits intomainfrom
fix/095-squid-non-root

Conversation

@Mossaka
Copy link
Collaborator

@Mossaka Mossaka commented Mar 12, 2026

Summary

  • Add USER proxy directive to Squid Dockerfile so the container runs as the non-root proxy user (uid=13) from the start
  • Remove gosu dependency and root-only chown operations from entrypoint
  • Set proper ownership on mounted volumes (squid logs, SSL db) from the host side before container start
  • Add pid_filename to squid.conf pointing to proxy-owned /var/run/squid/ directory

Security Impact

Reduces blast radius of potential container escapes or Squid vulnerabilities. Previously the container started as root (using gosu to drop privileges for the main process). Now the entire container lifecycle runs as non-root.

Fixes #250

Test plan

  • Build passes (npm run build)
  • All 946 unit tests pass (npm test)
  • Lint passes (0 errors)
  • CI integration tests verify Squid container starts and proxies correctly
  • SSL Bump tests verify non-root container can access SSL db

🤖 Generated with Claude Code

Add USER directive to Squid Dockerfile so the container runs as the
non-root 'proxy' user (uid=13) from the start, reducing the impact
of potential container escapes or Squid vulnerabilities.

Changes:
- Dockerfile: Add USER proxy, remove gosu dependency
- entrypoint.sh: Remove root-only chown operations and gosu exec
- docker-manager.ts: Set squid log dir ownership to proxy uid
- ssl-bump.ts: Chown SSL db to proxy user after initialization
- squid-config.ts: Add pid_filename to proxy-owned directory

Fixes #250

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings March 12, 2026 23:51
@github-actions
Copy link
Contributor

⚠️ Coverage Regression Detected

This PR decreases test coverage. Please add tests to maintain coverage levels.

Overall Coverage

Metric Base PR Delta
Lines 84.23% 84.24% ➡️ +0.01%
Statements 84.21% 84.18% 📉 -0.03%
Functions 84.37% 84.44% 📈 +0.07%
Branches 77.09% 77.00% 📉 -0.09%
📁 Per-file Coverage Changes (2 files)
File Lines (Before → After) Statements (Before → After)
src/ssl-bump.ts 90.5% → 87.0% (-3.52%) 90.7% → 86.4% (-4.21%)
src/docker-manager.ts 86.8% → 87.4% (+0.61%) 86.1% → 86.7% (+0.59%)

Coverage comparison generated by scripts/ci/compare-coverage.ts

@Mossaka Mossaka changed the title fix(security): run Squid container as non-root proxy user fix(squid): run Squid container as non-root proxy user Mar 12, 2026
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Hardens the Squid proxy container by running it as the non-root proxy user from container start, shifting any required filesystem permission setup to image build time and/or host-side pre-configuration.

Changes:

  • Run the Squid container as USER proxy and remove gosu-based privilege dropping.
  • Move mounted-volume permission handling to the host side (e.g., create/chown Squid logs dir; chown SSL DB).
  • Update generated Squid config to place the PID file under a proxy-owned run directory.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/ssl-bump.ts Adds host-side recursive chown of the SSL DB so a non-root Squid can access the mounted directory.
src/squid-config.ts Sets pid_filename to /var/run/squid/squid.pid for non-root operation.
src/docker-manager.ts Adjusts Squid logs directory creation to prefer chown to UID/GID 13 with a chmod fallback.
containers/squid/entrypoint.sh Removes root-only permission fixes and starts Squid directly as the current user.
containers/squid/Dockerfile Removes gosu package and sets USER proxy for the container runtime user.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/ssl-bump.ts Outdated
Comment on lines +24 to +33
function chownRecursive(dirPath: string, uid: number, gid: number): void {
fs.chownSync(dirPath, uid, gid);
for (const entry of fs.readdirSync(dirPath, { withFileTypes: true })) {
const fullPath = path.join(dirPath, entry.name);
if (entry.isDirectory()) {
chownRecursive(fullPath, uid, gid);
} else {
fs.chownSync(fullPath, uid, gid);
}
}
Comment on lines +291 to +298
// Chown to proxy user (uid=13, gid=13) so the non-root Squid container can access it
// Gracefully skip if not running as root (e.g., in unit tests)
try {
chownRecursive(sslDbPath, 13, 13);
} catch (err: unknown) {
if ((err as NodeJS.ErrnoException).code !== 'EPERM') throw err;
logger.debug('Skipping SSL db chown (not running as root)');
}
Comment on lines +1199 to +1203
try {
fs.chownSync(squidLogsDir, SQUID_PROXY_UID, SQUID_PROXY_GID);
} catch {
// Fallback to world-writable if chown fails (e.g., non-root context)
fs.chmodSync(squidLogsDir, 0o777);
@github-actions
Copy link
Contributor

Smoke Test Results — PASS

Test Result
GitHub MCP (last 2 merged PRs) #1267 "fix: drop -f from curl to avoid GitHub API rate-limit flakiness", #1265 "fix: add missing formatItem and program imports in cli.test.ts"
Playwright (github.com title) ✅ Title contains "GitHub"
File writing /tmp/gh-aw/agent/smoke-test-copilot-23029427035.txt created
Bash verification ✅ File content confirmed

Author: @Mossaka | No assignees

📰 BREAKING: Report filed by Smoke Copilot for issue #1271

Export chownRecursive function and add isolated tests with mocked fs
to cover directory traversal logic. Add tests for EPERM graceful
handling in initSslDb. Fixes coverage regression in ssl-bump.ts.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@github-actions
Copy link
Contributor

Smoke Test Results

PRs: feat(proxy): add GitHub Enterprise Cloud/Server support (#1264), feat(cli): add predownload command to pre-pull container images (#1245)

  • ✅ GitHub MCP: 2 merged PRs retrieved
  • ✅ Playwright: GitHub page title verified
  • ✅ File write: /tmp/gh-aw/agent/smoke-test-claude-23029427070.txt created
  • ✅ Bash: File contents confirmed

Overall: PASS

💥 [THE END] — Illustrated by Smoke Claude for issue #1271

@github-actions
Copy link
Contributor

✅ Coverage Check Passed

Overall Coverage

Metric Base PR Delta
Lines 84.23% 84.46% 📈 +0.23%
Statements 84.21% 84.40% 📈 +0.19%
Functions 84.37% 84.44% 📈 +0.07%
Branches 77.09% 77.16% 📈 +0.07%
📁 Per-file Coverage Changes (2 files)
File Lines (Before → After) Statements (Before → After)
src/docker-manager.ts 86.8% → 87.4% (+0.61%) 86.1% → 86.7% (+0.59%)
src/ssl-bump.ts 90.5% → 91.4% (+0.90%) 90.7% → 90.8% (+0.10%)

Coverage comparison generated by scripts/ci/compare-coverage.ts

@github-actions
Copy link
Contributor

Smoke Test Results@Mossaka

✅ GitHub MCP: #1267 "fix: drop -f from curl to avoid GitHub API rate-limit flakiness", #1265 "fix: add missing formatItem and program imports in cli.test.ts"
✅ Playwright: github.com title contains "GitHub"
✅ File write: /tmp/gh-aw/agent/smoke-test-copilot-23029571843.txt created
✅ Bash: file read back successfully

Overall: PASS

📰 BREAKING: Report filed by Smoke Copilot for issue #1271

@github-actions
Copy link
Contributor

Smoke Test Results

GitHub MCP: #1264 feat(proxy): add GitHub Enterprise Cloud/Server support | #1267 fix: drop -f from curl to avoid GitHub API rate-limit flakiness
Playwright: github.com title contains "GitHub"
File Write: /tmp/gh-aw/agent/smoke-test-claude-23029571826.txt created
Bash: File contents verified

Overall: PASS

💥 [THE END] — Illustrated by Smoke Claude for issue #1271

@github-actions
Copy link
Contributor

fix(squid): run Squid container as non-root proxy user
fix(docker): simplify to localhost+Squid-only iptables
Tests: MCP✅ GH✅ Playwright✅ Tavily❌ File✅ Bash✅ Discussion✅ Build✅
Overall: FAIL

🔮 The oracle has spoken through Smoke Codex for issue #1271

@github-actions
Copy link
Contributor

Chroot Version Comparison Results

Runtime Host Version Chroot Version Match?
Python Python 3.12.12 Python 3.12.3 ❌ NO
Node.js v24.14.0 v20.20.0 ❌ NO
Go go1.22.12 go1.22.12 ✅ YES

Overall: ❌ FAILED — Python and Node.js versions differ between host and chroot environments.

Tested by Smoke Chroot for issue #1271

@github-actions
Copy link
Contributor

🏗️ Build Test Suite Results

Ecosystem Project Build/Install Tests Status
Bun elysia 1/1 passed ✅ PASS
Bun hono 1/1 passed ✅ PASS
C++ fmt N/A ✅ PASS
C++ json N/A ✅ PASS
Deno oak N/A 1/1 passed ✅ PASS
Deno std N/A 1/1 passed ✅ PASS
.NET hello-world N/A ✅ PASS
.NET json-parse N/A ✅ PASS
Go color passed ✅ PASS
Go env passed ✅ PASS
Go uuid passed ✅ PASS
Java gson N/A ❌ FAIL
Java caffeine N/A ❌ FAIL
Node.js clsx passed ✅ PASS
Node.js execa passed ✅ PASS
Node.js p-limit passed ✅ PASS
Rust fd 1/1 passed ✅ PASS
Rust zoxide 1/1 passed ✅ PASS

Overall: 7/8 ecosystems passed — ❌ FAIL


❌ Failure Details

Java (gson, caffeine) — Maven could not download dependencies from Maven Central.

Error: Could not transfer artifact ... from/to central (https://repo.maven.apache.org/maven2): Network is unreachable

Root cause: The runner environment has an egress firewall (AWF). Maven attempted to use the configured proxy (squid-proxy:3128 via JAVA_TOOL_OPTIONS), but squid-proxy is not resolvable in this context (it is only available inside the AWF Docker network). Without the proxy, direct HTTPS access to repo.maven.apache.org is blocked by the network firewall. Neither the proxy-based nor the direct network path was available to Maven in this runner.

Generated by Build Test Suite for issue #1271 ·

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Security] Run Squid container as non-root user

2 participants