fix: chmod mkdtempSync dirs and add file-permissions-checker CI#6311
Conversation
The EACCES fallback in hosts-file.ts (PR #6285) still crashed because the fallback path also used mkdtempSync without overriding the restrictive umask (0177) that some Ubuntu 24.04 runners apply. Both the primary and fallback mkdtempSync now get an explicit chmodSync(dir, 0o700). Additionally, adds a comprehensive file-permissions-checker CI workflow that catches this class of bug via: - Static analysis: detects mkdtempSync without chmod, mkdirSync mode without chmod, unguarded cleanup file ops, execa chmod without error handling - Runtime tests: validates all critical directory creation patterns under a simulated restrictive umask (0177) Closes github/gh-aw#46062 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8bef4805-c013-4d25-908f-125577735990
The cross-uid-cleanup-resilience test creates a directory then writes a file into it. Under umask 0177 (as set by the CI step), mkdirSync creates the dir with mode 0600 (no execute bit), so the writeFileSync fails before we even get to restrict the dir to 0555. Fix: chmod the dir to 0755 immediately after creation, then restrict it after writing the test file. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8bef4805-c013-4d25-908f-125577735990
✅ Coverage Check PassedOverall Coverage
📁 Per-file Coverage Changes (1 files)
Coverage comparison generated by |
There was a problem hiding this comment.
Pull request overview
Fixes restrictive-umask failures when creating chroot hosts files and adds CI permission checks.
Changes:
- Explicitly applies
0700permissions to primary and fallback temporary directories. - Adds static permission analysis and runtime checks.
- Adds a dedicated CI workflow.
Show a summary per file
| File | Description |
|---|---|
src/services/agent-volumes/hosts-file.ts |
Hardens temporary directory permissions. |
scripts/ci/check-file-permissions.ts |
Adds static and runtime permission checks. |
.github/workflows/file-permissions-checker.yml |
Runs the new checker in CI. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 3/3 changed files
- Comments generated: 5
- Review effort level: Medium
| jobs: | ||
| file-permissions-check: | ||
| name: Check File Permission Patterns | ||
| runs-on: ubuntu-latest |
| // Extract the variable name assigned from mkdtempSync | ||
| const assignMatch = line.match(/(?:const|let|var)\s+(\w+)\s*=\s*(?:fs\.)?mkdtempSync/); | ||
| if (!assignMatch) continue; |
| fs.unlinkSync(testFile); | ||
| return { | ||
| name: 'mkdtempSync-without-chmod-fails (umask enforcement)', | ||
| passed: true, |
| // Create a chroot subdir like hosts-file.ts does | ||
| const chrootDir = fs.mkdtempSync(path.join(workDir, 'chroot-')); | ||
| fs.chmodSync(chrootDir, 0o700); |
| // Create a subdirectory and remove write permission (simulates root-owned dir) | ||
| const restrictedDir = path.join(testDir, 'restricted'); | ||
| fs.mkdirSync(restrictedDir); | ||
| fs.writeFileSync(path.join(restrictedDir, 'file.txt'), 'data'); | ||
| fs.chmodSync(restrictedDir, 0o555); // read+execute only |
- Wire matrix.os to runs-on so ubuntu-24.04 actually runs on that image - Handle multiline mkdtempSync assignments in static analysis (look back up to 3 lines for split const/let declarations) - Negative test now fails on Linux if EACCES can't be reproduced (validates the threat model is testable on the CI environment) - Integration test imports actual built generateHostsFileMount function instead of reimplementing the pattern inline - Rename cross-uid test to chmod-repair-retry-pattern (honest about what it tests: the local chmod+retry logic, not Docker-based repair) - Add explicit mode to all writeFileSync calls (fixes CodeQL js/insecure-temporary-file alert) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8bef4805-c013-4d25-908f-125577735990
|
✅ Copilot review passed with no inline comments. @lpcox Add the |
|
✅ Smoke Copilot BYOK completed. Copilot BYOK mode operational. 🔓 |
|
✅ Smoke Gemini completed. All facets verified. 💎 |
|
✅ Build Test Suite completed successfully! |
|
📰 VERDICT: Smoke Docker Sbx has concluded. All systems operational. This is a developing story. 🎤 |
|
❌ Smoke Copilot BYOK AOAI (api-key) reports failed. AOAI BYOK (api-key) mode investigation needed... |
|
Chroot tests passed! Smoke Chroot - All security and functionality tests succeeded. |
|
🔑 Smoke Copilot PAT PAT auth validated. All systems operational. ✅ |
|
✅ Contribution Check completed successfully! PR #6311 follows the applicable CONTRIBUTING.md guidelines: clear description with related references, appropriate file organization, tests/runtime coverage for the permission fix, and no documentation update is required for this internal bug fix. |
|
✅ Smoke Claude passed |
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
|
❌ Smoke Copilot BYOK AOAI (Entra) reports failed. AOAI BYOK (Entra) mode investigation needed... |
|
🔌 Smoke Services — All services reachable! ✅ |
|
✨ The prophecy is fulfilled... Smoke Codex has completed its mystical journey. The stars align. 🌟 |
|
🚀 Security Guard has started processing this pull request |
|
📡 Smoke OTel Tracing completed. All tracing scenarios validated. ✅ |
Smoke Test: Claude Engine Validation
Overall Result: PASS Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "awmgmcpg"See Network Configuration for more information.
|
Smoke Test: PAT Auth Validation
Overall: PASS Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "awmgmcpg"See Network Configuration for more information.
|
🔬 Smoke Test Results
Overall: PASS (engine reachable; template vars not resolved in this run) @lpcox — smoke test complete. Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "awmgmcpg"See Network Configuration for more information.
|
Smoke Test: Copilot BYOK (Direct) Mode
Running in direct BYOK mode ( Overall: PASS Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "awmgmcpg"See Network Configuration for more information.
|
Gemini Smoke Test Results
Overall Status: FAIL Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "localhost"See Network Configuration for more information.
|
Smoke Test: GitHub Actions Services Connectivity
Overall: FAIL — service containers are not reachable from this runner environment. Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "awmgmcpg"See Network Configuration for more information.
|
🔍 Smoke Test: API Proxy OpenTelemetry Tracing
Summary: All 4 directly testable scenarios pass. Scenario 5 (live span export) requires a running container and is expected-pending in this sandbox. No unexpected failures. Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "awmgmcpg"See Network Configuration for more information.
|
🔍 Smoke Test: Docker Sbx Validation
Overall: PARTIAL — MCP connectivity confirmed; pre-computed step outputs were not injected (template substitution failed in workflow).
|
Chroot Version Comparison Results
ALL_TESTS_PASSED: false — Python and Node.js versions differ between host and chroot environments. Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "awmgmcpg"See Network Configuration for more information.
|
🏗️ Build Test Suite Results
Overall: 8/8 ecosystems passed — ✅ PASS Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "awmgmcpg"See Network Configuration for more information.
|
Smoke TestDeduplicate OIDC test env isolation across API proxy config suites ✅ Warning Firewall blocked 2 domainsThe following domains were blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "awmgmcpg"
- "registry.npmjs.org"See Network Configuration for more information.
|
Problem
Smoke CI run 29526686386 fails with:
PR #6285 (merged today) added a diagnostic + fallback for the initial EACCES, but the fallback itself uses the same broken
mkdtempSyncpattern — creating a new temp dir that also getsmode=600from the runner's restrictive umask (0177).Root Cause
On Ubuntu 24.04 runners (
ubuntu24/20260714.240), the process umask is0177. Node.jsmkdtempSyncapplies the umask to the default0700mode, resulting in directories with0600(missing the execute bit). Without the execute bit, no files can be created inside the directory → EACCES.Fix
1.
src/services/agent-volumes/hosts-file.tsAdded
fs.chmodSync(dir, 0o700)after bothmkdtempSynccalls (primary path and fallback).chmodSyncis not affected by umask — it sets the exact mode.2. New CI:
.github/workflows/file-permissions-checker.ymlComprehensive checker that prevents this entire class of bug from recurring:
Static analysis (errors = CI fails):
mkdtempSyncwithout a followingchmodSyncStatic analysis (warnings = informational):
mkdirSyncwith mode but nochmodSyncbackupexeca.sync('chmod')without error handlingRuntime tests (8 tests, run under simulated umask
0177):Runs on both
ubuntu-latestandubuntu-24.04matrices and under forced restrictive umask.Testing
hosts-filetests passRelated