[loks0n]

Security: the executor follows symlinks a runtime or build plants

The executor reads several files from directories that a runtime or build container mounts read-write. Every one of those reads followed symlinks:

Read	Container path	What a planted link returns
Execution logs (createExecution)	/mnt/logs/<id>_logs.log, _errors.log	the target, as the execution's logs
Build logs (Logs::get, getLogs stream)	/tmp/logging/logs.txt	the target, as build logs
Build output	/mnt/code/code.tar.gz (or .tar, .sqfs, .erofs)	the target, uploaded as the deployment, which the owner can download
Build cache	/cache/stores.sqfs	the target, uploaded as the project's build cache

The execution log id also comes straight from the runtime's x-open-runtimes-log-id response header, unchecked, so ../ in it walked anywhere the executor can read.

A function that replaces its log file with ln -sf /proc/self/environ gets the executor's environment back as its logs, including OPR_EXECUTOR_SECRET. With that secret, code running in a function can call the executor API itself. On a self-hosted install, the executor drives Docker, so that is very likely root on the host. Any install where people who aren't fully trusted can deploy functions or sites is exposed.

This is the same class of bug as one exploited against Appwrite Cloud's executor on 1 November, which has been fixed there.

Fix

• RegularFile reads only an entry that lstat says is a regular file, and only if opening it non-blocking ('rbn') lands on the same device and inode. So a symlink, FIFO, device or directory reads as empty, and so does a file swapped for a link between the check and the open. A FIFO can't hang the executor.
• Execution logs go through RegularFile::read and are then unlinked. Unlinking a symlink only removes the link, never its target. The log id must be alphanumeric. The runtimes generate hex ids, so nothing legitimate changes.
• Build logs: Logs::get and the getLogs stream read logs.txt through RegularFile.
• Build output: anything but a regular file fails the build, the same way a missing output always has. The build container is still running during the upload, so RegularFile::copy copies the output from the handle that passed the checks into /tmp/<runtime>/, which no container mounts, and the upload reads that copy, which is removed afterwards. The path is never reopened by name, so a swap after the check can't change what gets uploaded.
• Build cache: a non-regular stores.sqfs is skipped. Otherwise it's uploaded from a private copy, the same way as the build output.

Tests

• RegularFileTest covers:
  • a regular file is read, with offset and length;
  • a planted symlink reads as empty, and its target is left alone;
  • a FIFO is refused without blocking;
  • a missing path reads as empty;
  • copy copies a regular file, refuses a symlink, and won't write over an entry already at its destination.
• testBuildLogsDoNotFollowASymlinkTheBuildPlanted drives Logs::get, the function the build path calls.
• Seen red: with the old Logs.php restored, that test fails and returns OPR_EXECUTOR_SECRET=leaked as build log content. With lstat swapped for stat in RegularFile, the symlink read and copy tests fail.
• E2E, against the real executor:
  • testExecutionLogSymlinkIsNotFollowed: a function swaps its log files for links to /proc/self/environ, and the test asserts no OPR_EXECUTOR_SECRET comes back.
  • testBuildOutputSymlinkIsNotUploaded: a build leaves a symlink as code.tar.gz, and the test asserts nothing is uploaded.
  • testBuildCacheSymlinkIsNotStored: a build swaps its cache artifact for a symlink, and a second build with the same key prints what was restored.
  • Seen red: against an executor built from main, the build test gets a 201, the log test returns the executor's environment (test secret included) as logs, and the cache test restores and prints it.

composer test:unit, format:check, analyze, refactor:check the three new E2E tests and testBuildCache pass locally.

After merge

Release, then bump the executor in appwrite/appwrite so self-hosted installs get it.

🤖 Generated with Claude Code

[hansi-codes]

🟢 Tier S · Ready to merge

Adds a regular-file-only reader and copier for files writable by runtime or build containers, then uses them for execution logs, build logs, build outputs, and cache artifacts. Adds unit and end-to-end coverage for planted symlink cases.

Verdict	New comments	Fixed	Still open
✅ Approved	0	0	0

📂 Walkthrough · 7

File	Change
src/Executor/Logs.php	Reads build log and timing files through the regular-file helper.
src/Executor/RegularFile.php	Adds guarded reads and private-copy support for regular files.
src/Executor/Runner/Docker.php	Uses guarded file handling for streamed logs, execution logs, build output, and cache uploads.
tests/e2e/ExecutorTest.php	Adds end-to-end symlink cases for build output, execution logs, and cache artifacts.
tests/resources/functions/node-log-symlink/index.js	Adds a function fixture that replaces its log files with symlinks.
tests/resources/functions/node-log-symlink/package.json	Adds package metadata for the execution-log test fixture.
tests/unit/Executor/RegularFileTest.php	Tests guarded file reads/copies and build-log symlink handling.

🔇 Filtered out · 1

Findings Hansi considered but did not post.

Finding

Why

Execution-log test targets a different runtime ID

Verifier: The executions endpoint lazily creates missing runtimes when image and source are supplied (src/Executor/Runner/Docker.php:723–795). This test supplies both, along with entrypoint and runtimeEntrypoint, so it creates test-exec-log-symlink using the returned build artifact and executes the function. Different build and execution IDs are intentional and also used in testBuildUncompressed. The suggested replacement is unnecessary and would leave the DELETE at line 486 targeting a different runtime from the one created.

---

_{Reviewed a8a8a2f · Details · Comment @hansi-codes review to re-run, or mention @hansi-codes with a question.}

[hansi-codes]

🔴 Tier D · 1 blocking finding to address. Summary

[loks0n]

@hansi-codes review

The findings that requested changes are resolved. See the summary comment for what is still open.

[hansi-codes]

🟢 Tier S · Looks good to merge. Summary

[hansi-codes]

🔵 Tier A · Looks good to merge. Summary

[loks0n]

@hansi-codes review

[hansi-codes]

🔵 Tier A · Looks good to merge. Summary

[loks0n]

@hansi-codes review

[hansi-codes]

🟢 Tier S · Looks good to merge. Summary

[hansi-codes]

🟢 Tier S · Looks good to merge. Summary

[loks0n]

Published executor 1.3.1 from merge commit 631d487e4a0bd68c35bd88725b63868b92d546cb. The release workflow succeeded; Docker Hub serves linux/amd64 and linux/arm64, index digest sha256:32fd5fd1a31c7280669250a74494d9965efad64da8b2ac478ff9bf4c97e2e27a.

Appwrite adoption is in appwrite/appwrite#15310, assigned to @loks0n. Compose validation and diff checks pass. Its CI/review and merge are the remaining consumer steps.