Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 47 additions & 0 deletions backend/tests/test_umask_security.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,3 +91,50 @@ def test_repair_data_permissions(tmp_path, monkeypatch):
assert (secret_file.stat().st_mode & 0o777) == 0o600

_reset_path_manager_singleton()


def test_repair_data_permissions_does_not_follow_symlinks(tmp_path, monkeypatch):
"""A link inside the data directory must not re-permission its target.

Regression test for issue #164: the Codex CLI persists argv[0] dispatch
links under the data volume that point at its own binary inside the image,
and the repair pass chmod-ed the binary to 0600 through them, leaving it
unexecutable for the worker.
"""
_reset_path_manager_singleton()
monkeypatch.setattr(PathManager, "_get_project_root", lambda self: tmp_path)
monkeypatch.setattr(PathManager, "_is_containerized_runtime", lambda self: True)

manager = PathManager()
user_data_dir = tmp_path / "data"
user_data_dir.mkdir(parents=True)
monkeypatch.setattr(manager, "_user_data_directory", user_data_dir)

# Targets outside the data directory, standing in for files in the image.
outside = tmp_path / "outside"
outside.mkdir()
outside_binary = outside / "codex"
outside_binary.write_text("#!/bin/sh\n", encoding="utf-8")
outside_binary.chmod(0o755)
outside_dir = outside / "vendor"
outside_dir.mkdir()
outside_dir.chmod(0o755)

# Links persisted inside the data directory, as the Codex CLI leaves them.
arg0_dir = user_data_dir / "cli-oauth" / "1" / "codex" / "tmp" / "arg0"
arg0_dir.mkdir(parents=True)
file_link = arg0_dir / "apply_patch"
file_link.symlink_to(outside_binary)
dir_link = arg0_dir / "vendor"
dir_link.symlink_to(outside_dir, target_is_directory=True)

manager.repair_data_permissions()

# Link targets are untouched, so the binary stays executable.
assert (outside_binary.stat().st_mode & 0o777) == 0o755
assert (outside_dir.stat().st_mode & 0o777) == 0o755

# Real entries in the same tree are still repaired.
assert (arg0_dir.stat().st_mode & 0o777) == 0o700

_reset_path_manager_singleton()
21 changes: 19 additions & 2 deletions backend/utils/path_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,17 @@ def repair_data_permissions(self) -> None:
Recursively scans the user data directory and enforces:
- 0700 (owner read/write/execute) on all directories.
- 0600 (owner read/write) on all files.

Symbolic links found in the tree are skipped rather than re-permissioned.
``chmod`` dereferences, and ``os.walk`` yields a link-to-file under
``files`` and a link-to-directory under ``dirs``, so a link persisted
inside the data directory would otherwise have its *target* set to
0600/0700 — anywhere on the filesystem. That is how issue #164 left the
bundled Codex binary at 0600 and unexecutable: the Codex CLI keeps
argv[0] dispatch links under ``CODEX_HOME/tmp/arg0`` pointing back at
itself inside the image, and ``CODEX_HOME`` lives on the persistent data
volume. Skipping is the only option here — Linux has no ``lchmod``, so
``os.chmod(..., follow_symlinks=False)`` raises ``NotImplementedError``.
"""
user_data_dir = self._user_data_directory
if not user_data_dir.exists():
Expand All @@ -211,16 +222,22 @@ def repair_data_permissions(self) -> None:

# Walk the directory structure recursively
for root, dirs, files in os.walk(user_data_dir):
root_path = Path(root)
# Pruned in place so the walk neither descends into a symlinked
# directory nor chmods it below (which would follow the link).
dirs[:] = [d for d in dirs if not (root_path / d).is_symlink()]
for d in dirs:
dir_path = Path(root) / d
dir_path = root_path / d
try:
dir_path.chmod(0o700)
except OSError as e:
logger.warning(
"Could not set permissions on directory %s: %s", dir_path, e
)
for f in files:
file_path = Path(root) / f
file_path = root_path / f
if file_path.is_symlink():
continue
try:
file_path.chmod(0o600)
except OSError as e:
Expand Down
2 changes: 1 addition & 1 deletion docs/DEPLOYMENT.md
Original file line number Diff line number Diff line change
Expand Up @@ -688,7 +688,7 @@ Pinning a deployment to an exact image digest (`ghcr.io/valtora/nojoin-api@sha25
```bash
chmod 600 nginx/cert.key
```
- **Confidential Data File Permissions (SEC-006):** For security hardening, all confidential application data files (audio recordings, JWT keys, logs, documents, configuration files) now default to owner-only permissions. A recursive startup repair pass automatically secures existing data inside the container-mounted directory. If you are using host-mounted directories and want to align host-level permissions, you can manually restrict them:
- **Confidential Data File Permissions (SEC-006):** For security hardening, all confidential application data files (audio recordings, JWT keys, logs, documents, configuration files) now default to owner-only permissions. A recursive startup repair pass automatically secures existing data inside the container-mounted directory. The pass skips symbolic links, so a link stored under the data directory never has its target re-permissioned elsewhere on the filesystem. If you are using host-mounted directories and want to align host-level permissions, you can manually restrict them:
```bash
chmod -R 700 ./data
```
Expand Down
Loading