runner: Never let a job .blend read host files outside its context #47

Merged

linus merged 1 commit from ash/relink-host-paths-43 into develop

2026-11-03 16:18:03 +00:00

ash commented

2026-11-03 15:58:29 +00:00

Collaborator

A job .blend could make the runner read any file the runner user can read and send it back in the render. Blake found it in the security review of PR 42. This does both issues in one PR, since they touch the same file (Mira's call allowed that).

What changes

An external file that is outside the job context and has no match there is pointed at <job>/.not-in-context/<name>, which doesn't exist, and reloaded. Blender then has no data for it, and the task log names it.
An unmatched library outside the context is removed, along with everything linked from it. Blender reads libraries when it opens the .blend, before the script runs, so pointing the path somewhere else and reloading is not enough: the linked data stays in memory (the real Blender run below shows this).
Linked IDs inside a library were skipped before. Now they are resolved against their own library and handled the same way. An in-context library could otherwise carry an image with an absolute host path.
_inside compares realpaths, and build_context_index skips symlinks and anything whose realpath leaves the root.
If relink_to_context raises, the render script exits with an error. Before, it printed a warning and rendered with the host paths.
A missing file whose path is inside the context is left as it is.

Not done: the optional ExtractTar change from #44 (reject link entries instead of dropping them). Links already never get extracted, and failing uploads that contain links is a product call. I can file it if you want it.

Proof

go test ./pkg/scripts/... -count=1 passes, with 13 Python tests. Four of the new tests fail on develop's relink_context.py: POSIX, //.. hops, X:\ and host library neutralised; linked image resolved against its library; symlink not indexed; symlink not relinked to.
go test ./... passes on the jumpbox except internal/runner/encoding, which needs ffmpeg (the jumpbox has none; CI does).
scripts/check-comments.py develop HEAD is ok.
Real Blender 5.2.3 on the jumpbox, opening a .blend with a host image and a host library, and a second .blend whose in-context library links an image at a host path. Output of the script, new code and then develop's code:

=== job (new)
BEFORE {'images': [('secret.png', '/tmp/jb43/host/secret.png', 'local', 'HAS_DATA')], 'libs': ['/tmp/jb43/host/host_lib.blend'], 'objects': ['Secret']}
AFTER  {'images': [('secret.png', '//.not-in-context/secret.png', 'local', 'no-data')], 'libs': [], 'objects': []}
=== job (develop)
AFTER  {'images': [('secret.png', '/tmp/jb43/host/secret.png', 'local', 'HAS_DATA')], 'libs': ['/tmp/jb43/host/host_lib.blend'], 'objects': ['Secret']}
=== job2, in-context library linking a host image (new)
AFTER  {'images': [('secret.png', '/tmp/jb43/job2/.not-in-context/secret.png', 'linked', 'no-data')], 'libs': ['//lib.blend'], 'objects': ['LeakObj']}
=== job2 (develop)
AFTER  {'images': [('secret.png', '/tmp/jb43/host/secret.png', 'linked', 'HAS_DATA')], 'libs': ['//lib.blend'], 'objects': ['LeakObj']}
=== job with copies in the context (new, the PR 42 path still works)
AFTER  {'images': [('secret.png', '//tex/secret.png', 'local', 'HAS_DATA')], 'libs': ['//lib/host_lib.blend'], 'objects': ['Secret']}

Closes #43
Closes #44

A job `.blend` could make the runner read any file the runner user can read and send it back in the render. Blake found it in the security review of [PR 42](https://git.s1d3sw1ped.com/s1d3sw1ped/jiggablend/pulls/42). This does both issues in one PR, since they touch the same file (Mira's call allowed that). ## What changes - An external file that is outside the job context and has no match there is pointed at `<job>/.not-in-context/<name>`, which doesn't exist, and reloaded. Blender then has no data for it, and the task log names it. - An unmatched library outside the context is removed, along with everything linked from it. Blender reads libraries when it opens the `.blend`, before the script runs, so pointing the path somewhere else and reloading is not enough: the linked data stays in memory (the real Blender run below shows this). - Linked IDs inside a library were skipped before. Now they are resolved against their own library and handled the same way. An in-context library could otherwise carry an image with an absolute host path. - `_inside` compares realpaths, and `build_context_index` skips symlinks and anything whose realpath leaves the root. - If `relink_to_context` raises, the render script exits with an error. Before, it printed a warning and rendered with the host paths. - A missing file whose path is inside the context is left as it is. Not done: the optional `ExtractTar` change from #44 (reject link entries instead of dropping them). Links already never get extracted, and failing uploads that contain links is a product call. I can file it if you want it. ## Proof - `go test ./pkg/scripts/... -count=1` passes, with 13 Python tests. Four of the new tests fail on develop's `relink_context.py`: POSIX, `//..` hops, `X:\` and host library neutralised; linked image resolved against its library; symlink not indexed; symlink not relinked to. - `go test ./...` passes on the jumpbox except `internal/runner/encoding`, which needs ffmpeg (the jumpbox has none; CI does). - `scripts/check-comments.py develop HEAD` is ok. - Real Blender 5.2.3 on the jumpbox, opening a `.blend` with a host image and a host library, and a second `.blend` whose in-context library links an image at a host path. Output of the script, new code and then develop's code: ``` === job (new) BEFORE {'images': [('secret.png', '/tmp/jb43/host/secret.png', 'local', 'HAS_DATA')], 'libs': ['/tmp/jb43/host/host_lib.blend'], 'objects': ['Secret']} AFTER {'images': [('secret.png', '//.not-in-context/secret.png', 'local', 'no-data')], 'libs': [], 'objects': []} === job (develop) AFTER {'images': [('secret.png', '/tmp/jb43/host/secret.png', 'local', 'HAS_DATA')], 'libs': ['/tmp/jb43/host/host_lib.blend'], 'objects': ['Secret']} === job2, in-context library linking a host image (new) AFTER {'images': [('secret.png', '/tmp/jb43/job2/.not-in-context/secret.png', 'linked', 'no-data')], 'libs': ['//lib.blend'], 'objects': ['LeakObj']} === job2 (develop) AFTER {'images': [('secret.png', '/tmp/jb43/host/secret.png', 'linked', 'HAS_DATA')], 'libs': ['//lib.blend'], 'objects': ['LeakObj']} === job with copies in the context (new, the PR 42 path still works) AFTER {'images': [('secret.png', '//tex/secret.png', 'local', 'HAS_DATA')], 'libs': ['//lib/host_lib.blend'], 'objects': ['Secret']} ``` Closes #43 Closes #44

ash self-assigned this

2026-11-03 15:58:29 +00:00

ash added 1 commit

2026-11-03 15:58:29 +00:00

runner: Never let a job .blend read host files outside its context (closes #43 , closes #44 )

Comments / comments (pull_request) Successful in 6s

Details

CI / check-and-test (pull_request) Successful in 1m21s

Details

1fbc25d157

relink_to_context left any external file it could not match in the job
context pointing at its original path, so a job could make the runner
read /etc/passwd, a home directory file or an X:\ path and send it back
in the render. Linked IDs inside a library were skipped entirely, and a
symlink in the context that pointed outside it passed the inside check.

Now an unmatched file outside the context is pointed at a nonexistent
path under .not-in-context/ in the job dir and reloaded, so Blender has
no data for it. An unmatched library outside the context is removed
with everything linked from it, because Blender already read it when it
opened the .blend. Linked IDs are resolved against their own library
and get the same treatment. The inside check compares realpaths, and
the context index skips symlinks and anything whose realpath leaves the
root. If relinking itself raises, the render script now exits instead
of rendering with host paths.

Files that are missing but inside the context are left as they were.

ash requested review from blake

2026-11-03 15:58:29 +00:00

eva approved these changes

2026-11-03 16:00:43 +00:00

eva left a comment

GO from me on 1fbc25d.

I read the whole diff. It's on the current develop tip (d2f7276) and both CI checks are green.

What I checked:

An unmatched path outside the context now never stays pointed at the host. Images get the .not-in-context path and a reload, and host libraries are removed so their linked data goes with them. The real Blender run in the body shows the difference from develop.
Linked IDs inside a library now resolve against that library's path, which closes the in-context-library-with-a-host-image case.
_inside uses realpath on both sides and build_context_index skips symlinks, which covers #44's acceptance criteria, and the symlink tests prove it.
A relink failure now exits the render instead of rendering with host paths. sys is already imported in the template.
A missing file whose path is inside the context is left alone, as before.

One note, not a blocker: the commit closes #44, but its optional ExtractTar part isn't done. That's fine since it was marked optional; Mira can say whether she wants a follow-up issue.

This is security work (a file read through job uploads), so Blake still needs to review before Linus merges.

GO from me on 1fbc25d. I read the whole diff. It's on the current develop tip (d2f7276) and both CI checks are green. What I checked: - An unmatched path outside the context now never stays pointed at the host. Images get the `.not-in-context` path and a reload, and host libraries are removed so their linked data goes with them. The real Blender run in the body shows the difference from develop. - Linked IDs inside a library now resolve against that library's path, which closes the in-context-library-with-a-host-image case. - `_inside` uses realpath on both sides and `build_context_index` skips symlinks, which covers #44's acceptance criteria, and the symlink tests prove it. - A relink failure now exits the render instead of rendering with host paths. `sys` is already imported in the template. - A missing file whose path is inside the context is left alone, as before. One note, not a blocker: the commit closes #44, but its optional ExtractTar part isn't done. That's fine since it was marked optional; Mira can say whether she wants a follow-up issue. This is security work (a file read through job uploads), so Blake still needs to review before Linus merges.

blake approved these changes

2026-11-03 16:17:21 +00:00

blake left a comment

Security GO from me (Blake) on head 1fbc25d157, base develop d2f7276.

I read the full diff, the template, and how render.go fills in CONTEXT_DIR. I also ran relink_context_test.py at this head: all 13 tests pass.

What I checked:

Containment: _inside now runs realpath on both the path and the job folder before commonpath. That means symlinks are resolved before the check, and there is no prefix bug (/job and /job2 compare as separate folders).
Unmatched host paths (#43): when an absolute path, a //../.. path that climbs out, or a drive-letter path has no copy in the job folder, it is pointed at <job>/.not-in-context/<name> instead of being left on the host. Host libraries that are blocked are removed. Linked IDs now resolve against their own library, which closes the gap left by the old library is not None: continue.
Symlinks (#44): the index skips links and anything whose realpath is outside the job folder. os.walk does not follow directory links, and a link inside the job folder that points out is not relinked to.
If relinking fails, the render now exits instead of going on to read host paths. sys is already imported.
The job path is still put into the script with Go %q on the runner-owned WorkDir. That is unchanged, and the user can't control it.
No new dependencies, and no CI or secrets changes.

Still open for Mira, not a blocker: #44's optional part, rejecting symlinks in uploaded archives outright, is not done.

Security GO from me (Blake) on head 1fbc25d157, base develop d2f7276. I read the full diff, the template, and how render.go fills in CONTEXT_DIR. I also ran relink_context_test.py at this head: all 13 tests pass. What I checked: - Containment: `_inside` now runs realpath on both the path and the job folder before `commonpath`. That means symlinks are resolved before the check, and there is no prefix bug (/job and /job2 compare as separate folders). - Unmatched host paths (#43): when an absolute path, a `//../..` path that climbs out, or a drive-letter path has no copy in the job folder, it is pointed at `<job>/.not-in-context/<name>` instead of being left on the host. Host libraries that are blocked are removed. Linked IDs now resolve against their own library, which closes the gap left by the old `library is not None: continue`. - Symlinks (#44): the index skips links and anything whose realpath is outside the job folder. `os.walk` does not follow directory links, and a link inside the job folder that points out is not relinked to. - If relinking fails, the render now exits instead of going on to read host paths. `sys` is already imported. - The job path is still put into the script with Go `%q` on the runner-owned WorkDir. That is unchanged, and the user can't control it. - No new dependencies, and no CI or secrets changes. Still open for Mira, not a blocker: #44's optional part, rejecting symlinks in uploaded archives outright, is not done.