Fix local file handling and validation bugs - #58
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b314460053
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| relative_path = path.relative_to(workspace) | ||
| directory_flags = os.O_RDONLY | getattr(os, "O_DIRECTORY", 0) | getattr(os, "O_CLOEXEC", 0) | ||
| nofollow = getattr(os, "O_NOFOLLOW", 0) | ||
| directory_fd = os.open(workspace, directory_flags) |
There was a problem hiding this comment.
Anchor the workspace root without following symlinks
When another process renames the workspace directory itself after _resolve_output_path and replaces its original path with a symlink, this initial os.open(workspace, directory_flags) follows that symlink because O_NOFOLLOW is applied only to descendant components. I reproduced _write_workspace_file("out/secret", ...) successfully writing private data beneath an outside directory in this scenario, bypassing the intended workspace confinement. Open the current workspace descriptor without following its pathname (or use O_NOFOLLOW and verify its identity) before traversing components.
Useful? React with 👍 / 👎.
POSIX CLI writes could follow a replaced parent directory after path validation, and rejected API forms could leave unread uploads open. Anchor output creation, replacement, permissions, cleanup, and directory flushes to verified directory descriptors; close parsed uploads on every endpoint exit. Missing output directories now return
invalid_path, and overwrite temporary filenames support long destination names.The browser also retained the wrong file when selecting file A, dropping file B, then selecting A again. Reset the native picker after capturing its selection and count password length by Unicode code points to match Python validation. Add regression coverage and changelog entries for these fixes.
Validation on macOS with Python 3.13:
OQS_INSTALL_PATH="$PWD/.oqs" PYTHON=tmp/review-venv/bin/python ./test.sh tests/test_crypto_core.py tests/test_api_app.py tests/test_agent_tools.py tests/test_ui_helpers.py tests/test_startup.py: 368 passed, including native encryption/decryption.npm run check: TypeScript, 3 API-client tests, and 86 frontend unit tests passed.npm run build: passed.git diff --checkpassed.Native tests emitted a version warning because the available liboqs library is 0.15.0 and the Python wrapper is 0.16.0. The browser smoke attempt could not launch because the installed Playwright Chromium executable is missing. Windows behavior was not exercised locally; the existing fallback is preserved. Cryptographic algorithms and file formats are unchanged.
The development lock and CI compiler now use pip 26.2.1 with compatible pip-tools 7.6.1, addressing GHSA-qwm4-qh6w-59xr. Matching minimum versions prevent reinstating the vulnerable installer; checker expectations and setup instructions are synchronized. The local native install directory is ignored without deleting its files.
Dependency validation: hash-locked installation and pip check passed; the development-lock audit and npm audit reported no known vulnerabilities; 32 dependency-lock tests, Black, flake8, mypy, and Bandit passed. The two updated lock entries were generated with pip-compile on macOS and copied into the existing lock, preserving unchanged Linux-only entries because a local Linux runtime was unavailable. The complete canonical lock regeneration check passed in Ubuntu CI on the updated branch.