From fad4b958b77adf33b9a964d2d1f6d0e74b742b7a Mon Sep 17 00:00:00 2001 From: Salman Muin Kayser Chishti Date: Thu, 21 May 2026 16:55:29 +0000 Subject: [PATCH 1/2] fix(squid): chown bind-mounted log dirs to proxy user on startup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On split runner/Docker daemon filesystems (notably ARC + DinD), Docker auto-creates missing bind-mount source paths on the daemon side as root-owned. That bind-mount then overrides the Dockerfile-baked /var/log/squid (proxy-owned), and squid (UID 13) exits 1 the first time it tries to open access.log. The wrapper's runner-side mkdir + chown to UID 13 in config-writer.ts only applies to the runner's view of the source path — the daemon's view starts empty and is auto- created with root ownership. #3218 fixed the path-translation half of this class of bug. This change covers the second half: even when the path is correct, the daemon-side ownership has to be repaired before squid starts. The compose service builder now runs the squid container's entrypoint as root (user: '0:0'), performs a non-recursive chown of /var/log/squid (and /var/spool/squid_ssl_db when present) back to the proxy user, and drops to the proxy user via 'su -s /bin/bash proxy -c ...' before the image's own entrypoint runs. The image's entrypoint then handles the existing IPv6 listener strip and execs squid itself, all as proxy. Squid never runs as root; the root window is bounded to the two chown syscalls and the su exec. The chown is non-recursive: only the bind-mount source dir's own ownership needs repair, not its (potentially user-supplied) contents. This keeps the preflight bounded and fast even when proxyLogsDir points at a large host directory. su was chosen over runuser/gosu because the squid base image is plain ubuntu (util-linux ships in the base), so su is universally available without requiring an image rebuild. Doing the fix in the wrapper instead of in the squid image keeps it compatible with the older pinned squid shas that the integration and smoke tests use to validate the runtime stack against historical container artifacts. Tests: - Existing 'inject squid config via base64 env var' test still passes: the new entrypoint still emits the base64-decode, just wrapped after the chown and inside the su privilege drop. - New 'chown preflight + drop privileges' test asserts user '0:0', non-recursive chown of /var/log/squid and the conditional SSL DB chown, the ordering (chown before su), and that 'chown -R' is not used anywhere in the entrypoint. - New 'chown preflight applies without config' test covers the no-config branch. - All 2005 unit tests across the repo pass. --- src/services/squid-service.test.ts | 56 ++++++++++++++++++++++++++++++ src/services/squid-service.ts | 45 ++++++++++++++++++++---- 2 files changed, 95 insertions(+), 6 deletions(-) diff --git a/src/services/squid-service.test.ts b/src/services/squid-service.test.ts index 6c8c1d1ca..d9217894d 100644 --- a/src/services/squid-service.test.ts +++ b/src/services/squid-service.test.ts @@ -50,4 +50,60 @@ describe('squid service', () => { expect(squid.entrypoint[2]).toContain('base64 -d > /etc/squid/squid.conf'); expect(squid.entrypoint[2]).toContain('entrypoint.sh'); }); + + // Regression: on split runner/Docker daemon filesystems (ARC + DinD), Docker + // auto-creates missing bind-mount source dirs on the daemon side as root-owned. + // The bind-mount then overrides the Dockerfile-baked /var/log/squid (proxy- + // owned), and squid (UID 13) exits 1 the first time it tries to open + // access.log. The squid service must therefore start as root, chown the + // bind-mounted dir back to the proxy user, and drop privileges before squid + // runs. + it('should run squid container as root with a chown preflight that drops privileges', () => { + const squidConfig = 'http_port 3128\n'; + const result = generateDockerCompose(mockConfig, mockNetworkConfig, undefined, squidConfig); + const squid = result.services['squid-proxy'] as any; + + // The compose service must start as root so the preflight can chown + // bind-mounted paths it does not own. + expect(squid.user).toBe('0:0'); + + const inlineScript: string = squid.entrypoint[2]; + // Non-recursive chown on the dir only (NOT chown -R), so the preflight + // does not traverse a potentially large user-supplied proxyLogsDir. + expect(inlineScript).toMatch(/(^|[^R])chown proxy:proxy \/var\/log\/squid/); + expect(inlineScript).not.toContain('chown -R'); + // The SSL DB chown is conditional on the dir existing so it is a no-op + // when SSL Bump is disabled but engages automatically when it is enabled. + expect(inlineScript).toContain('if [ -d /var/spool/squid_ssl_db ]; then chown proxy:proxy /var/spool/squid_ssl_db; fi'); + // Privileges must drop before squid itself starts. We use su (always + // present in the ubuntu/squid base) rather than gosu or runuser. + expect(inlineScript).toContain('exec su -s /bin/bash proxy -c'); + + // The chown must precede the privilege drop. + const chownIdx = inlineScript.indexOf('chown proxy:proxy /var/log/squid'); + const suIdx = inlineScript.indexOf('exec su -s /bin/bash proxy -c'); + expect(chownIdx).toBeGreaterThanOrEqual(0); + expect(suIdx).toBeGreaterThan(chownIdx); + }); + + // The chown preflight is required regardless of whether squid config is + // injected, because the daemon-side ownership problem is independent of + // the config-injection mechanism. + it('should apply the chown preflight even when no squid config content is provided', () => { + const result = generateDockerCompose(mockConfig, mockNetworkConfig); + const squid = result.services['squid-proxy'] as any; + + expect(squid.user).toBe('0:0'); + expect(squid.entrypoint).toBeDefined(); + const inlineScript: string = squid.entrypoint[2]; + expect(inlineScript).toContain('chown proxy:proxy /var/log/squid'); + expect(inlineScript).not.toContain('chown -R'); + expect(inlineScript).toContain('exec su -s /bin/bash proxy -c'); + // Without injected config, the entrypoint should still hand off to the + // image's original entrypoint script (which handles IPv6 stripping etc.). + expect(inlineScript).toContain('/usr/local/bin/entrypoint.sh'); + // And it should NOT attempt to decode an AWF_SQUID_CONFIG_B64 that + // would be unset. + expect(inlineScript).not.toContain('AWF_SQUID_CONFIG_B64'); + }); }); diff --git a/src/services/squid-service.ts b/src/services/squid-service.ts index 89a9cf8be..12892dee7 100644 --- a/src/services/squid-service.ts +++ b/src/services/squid-service.ts @@ -99,20 +99,53 @@ export function buildSquidService(params: SquidServiceParams): any { // squid.conf fails because the daemon creates a directory at the missing path. // Passing the config as a base64-encoded env var works universally because // env vars are part of the container spec sent via the Docker API. + // + // The entrypoint also runs a chown preflight as root to repair the + // bind-mount source ownership on split runner/Docker daemon filesystems + // (e.g. ARC + DinD). The wrapper chowns /workDir/squid-logs to UID 13:13 + // in config-writer.ts, but only against the runner's view of the filesystem. + // On DinD the daemon's view of that path starts empty and Docker auto-creates + // it as root-owned, overriding the Dockerfile-baked /var/log/squid (proxy- + // owned) inside the container. Squid (UID 13) then exits 1 the first time it + // tries to open access.log. The non-recursive chown here repairs the dir's + // own ownership before squid starts. On shared-filesystem runners it is a + // no-op because the dir is already 13:13. After the chown the entrypoint + // drops to the proxy user via 'su -s /bin/bash proxy -c ...' before the + // image's own entrypoint script runs (which does the IPv6 strip and execs + // squid as the proxy user). + // + // su is used instead of runuser/gosu because the squid base image is plain + // ubuntu; su is in util-linux and present without any extra install. This + // keeps the change wrapper-only with no rebuild of the squid container. + // + // Use $$ to escape $ for Docker Compose variable interpolation. + // Docker Compose interprets $VAR as variable substitution in YAML values; + // $$ produces a literal $ that the shell inside the container will expand. + const SQUID_PROXY_USER = 'proxy'; + const chownPreflight = + `chown ${SQUID_PROXY_USER}:${SQUID_PROXY_USER} /var/log/squid && ` + + `if [ -d /var/spool/squid_ssl_db ]; then chown ${SQUID_PROXY_USER}:${SQUID_PROXY_USER} /var/spool/squid_ssl_db; fi`; + const dropToProxy = `exec su -s /bin/bash ${SQUID_PROXY_USER} -c`; + + squidService.user = '0:0'; if (squidConfigContent) { const configB64 = Buffer.from(squidConfigContent).toString('base64'); squidService.environment = { ...squidService.environment, AWF_SQUID_CONFIG_B64: configB64, }; - // Override entrypoint to decode the config before starting squid. - // The original entrypoint (/usr/local/bin/entrypoint.sh) is called after decoding. - // Use $$ to escape $ for Docker Compose variable interpolation. - // Docker Compose interprets $VAR as variable substitution in YAML values; - // $$ produces a literal $ that the shell inside the container will expand. + // After the chown, drop to proxy and decode the config there (so the + // resulting /etc/squid/squid.conf is proxy-owned and the image + // entrypoint's later sed -i succeeds), then exec the image entrypoint. + squidService.entrypoint = [ + '/bin/bash', '-c', + `${chownPreflight} && ${dropToProxy} 'echo "$$AWF_SQUID_CONFIG_B64" | base64 -d > /etc/squid/squid.conf && exec /usr/local/bin/entrypoint.sh'`, + ]; + } else { + // No config injection — just chown + drop + run the image entrypoint. squidService.entrypoint = [ '/bin/bash', '-c', - 'echo "$$AWF_SQUID_CONFIG_B64" | base64 -d > /etc/squid/squid.conf && exec /usr/local/bin/entrypoint.sh', + `${chownPreflight} && ${dropToProxy} 'exec /usr/local/bin/entrypoint.sh'`, ]; } From d0f447fa3129dbe79cbbeb354ff1e2598ada2985 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 21 May 2026 17:00:21 +0000 Subject: [PATCH 2/2] fix(squid): make chown preflight tolerant with chmod 0777 fallback --- src/services/squid-service.test.ts | 3 ++- src/services/squid-service.ts | 11 +++++++++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/src/services/squid-service.test.ts b/src/services/squid-service.test.ts index d9217894d..c2bc54add 100644 --- a/src/services/squid-service.test.ts +++ b/src/services/squid-service.test.ts @@ -74,7 +74,8 @@ describe('squid service', () => { expect(inlineScript).not.toContain('chown -R'); // The SSL DB chown is conditional on the dir existing so it is a no-op // when SSL Bump is disabled but engages automatically when it is enabled. - expect(inlineScript).toContain('if [ -d /var/spool/squid_ssl_db ]; then chown proxy:proxy /var/spool/squid_ssl_db; fi'); + // Falls back to chmod 0777 if chown is denied (tolerant, like config-writer.ts). + expect(inlineScript).toContain('if [ -d /var/spool/squid_ssl_db ]; then chown proxy:proxy /var/spool/squid_ssl_db 2>/dev/null || chmod 0777 /var/spool/squid_ssl_db; fi'); // Privileges must drop before squid itself starts. We use su (always // present in the ubuntu/squid base) rather than gosu or runuser. expect(inlineScript).toContain('exec su -s /bin/bash proxy -c'); diff --git a/src/services/squid-service.ts b/src/services/squid-service.ts index 12892dee7..0319e1bc0 100644 --- a/src/services/squid-service.ts +++ b/src/services/squid-service.ts @@ -118,13 +118,20 @@ export function buildSquidService(params: SquidServiceParams): any { // ubuntu; su is in util-linux and present without any extra install. This // keeps the change wrapper-only with no rebuild of the squid container. // + // The chown is tolerant: if chown fails (e.g. root-squash NFS, or the dir is + // already owned by the proxy user on a FS that denies root chown), we fall + // back to chmod 0777 — the same strategy as config-writer.ts — so the + // container does not exit when the directory is already writable. + // The chown is non-recursive (no -R): only the bind-mount dir's own + // ownership is repaired, not its (potentially large) contents. + // // Use $$ to escape $ for Docker Compose variable interpolation. // Docker Compose interprets $VAR as variable substitution in YAML values; // $$ produces a literal $ that the shell inside the container will expand. const SQUID_PROXY_USER = 'proxy'; const chownPreflight = - `chown ${SQUID_PROXY_USER}:${SQUID_PROXY_USER} /var/log/squid && ` + - `if [ -d /var/spool/squid_ssl_db ]; then chown ${SQUID_PROXY_USER}:${SQUID_PROXY_USER} /var/spool/squid_ssl_db; fi`; + `chown ${SQUID_PROXY_USER}:${SQUID_PROXY_USER} /var/log/squid 2>/dev/null || chmod 0777 /var/log/squid` + + `; if [ -d /var/spool/squid_ssl_db ]; then chown ${SQUID_PROXY_USER}:${SQUID_PROXY_USER} /var/spool/squid_ssl_db 2>/dev/null || chmod 0777 /var/spool/squid_ssl_db; fi`; const dropToProxy = `exec su -s /bin/bash ${SQUID_PROXY_USER} -c`; squidService.user = '0:0';