runner: Render from the job context, not absolute host paths (#41) #42

Merged
linus merged 1 commit from ash/windows-context-paths-41 into develop 2026-11-02 16:07:15 +00:00
Collaborator

What

The runner render script no longer calls make_paths_relative. It resolves every external file (libraries, images, movie clips, sounds, fonts, caches, volumes) against the extracted job context, matching by file name and the longest trailing run of path components (case-insensitive, UDIM/UVTILE aware). Matches are relinked relative to the blend and libraries are reloaded. Each file not in the context is logged by name, followed by one summary line:

Job context files: total N | already relative K | relinked R | not in context M

The runner's own output, home and sample_batches dirs are skipped when indexing the context.

Why

#41: Blender cannot make a path relative across drive letters, so on the ai1 Windows runner E:\ library paths stayed absolute (567 failed files), and on any host a surviving absolute path silently read host files instead of the job upload.

Test

  • 94 Go tests pass with -race in ./pkg/scripts and ./internal/runner/.... The one exception is the existing encoding integration test, which needs ffmpeg and fails on the jumpbox as before. New: TestRelinkContextPython (runs 8 Python unit tests for the resolver and relink pass) and TestCreateRenderScript_FillsEveryPlaceholder.
  • Real Blender 5.2.3 (Linux, Cycles CPU) render of a plane whose emission texture is red in the job context, using the script createRenderScript writes. The centre pixel was read from the EXR:
Scene texture path develop this branch
E:\Lib\Runtime\Textures\tex.png (absent on host) magenta (missing) red, relinked 1, not in context 0
absolute host path to a blue copy outside the context blue (host file) red (context file)
E:\Lib\Elsewhere\nothere.png (nowhere) magenta, silent magenta, Warning: Image 'seed.png' is not in the job context: E:/Lib/Elsewhere/nothere.png

The Windows check on ai1 is still needed: once this merges, Jerry reruns the job there.

Fixes #41

## What The runner render script no longer calls `make_paths_relative`. It resolves every external file (libraries, images, movie clips, sounds, fonts, caches, volumes) against the extracted job context, matching by file name and the longest trailing run of path components (case-insensitive, UDIM/UVTILE aware). Matches are relinked relative to the blend and libraries are reloaded. Each file not in the context is logged by name, followed by one summary line: `Job context files: total N | already relative K | relinked R | not in context M` The runner's own `output`, `home` and `sample_batches` dirs are skipped when indexing the context. ## Why https://git.s1d3sw1ped.com/s1d3sw1ped/jiggablend/issues/41: Blender cannot make a path relative across drive letters, so on the ai1 Windows runner `E:\` library paths stayed absolute (567 failed files), and on any host a surviving absolute path silently read host files instead of the job upload. ## Test - 94 Go tests pass with `-race` in `./pkg/scripts` and `./internal/runner/...`. The one exception is the existing encoding integration test, which needs ffmpeg and fails on the jumpbox as before. New: `TestRelinkContextPython` (runs 8 Python unit tests for the resolver and relink pass) and `TestCreateRenderScript_FillsEveryPlaceholder`. - Real Blender 5.2.3 (Linux, Cycles CPU) render of a plane whose emission texture is red in the job context, using the script `createRenderScript` writes. The centre pixel was read from the EXR: | Scene texture path | develop | this branch | |---|---|---| | `E:\Lib\Runtime\Textures\tex.png` (absent on host) | magenta (missing) | red, `relinked 1`, `not in context 0` | | absolute host path to a blue copy outside the context | blue (host file) | red (context file) | | `E:\Lib\Elsewhere\nothere.png` (nowhere) | magenta, silent | magenta, `Warning: Image 'seed.png' is not in the job context: E:/Lib/Elsewhere/nothere.png` | The Windows check on ai1 is still needed: once this merges, Jerry reruns the job there. Fixes #41
runner: Render from the job context, not absolute host paths (#41)
All checks were successful
CI / check-and-test (pull_request) Successful in 1m26s
adce7b258e
make_paths_relative cannot cross drive letters, so a Windows runner kept
E:\ library paths and silently read host files or failed them. The render
script now resolves every external file against the extracted job context,
relinks it relative to the blend, and names each file it cannot find.
ash requested review from eva 2026-11-02 16:01:10 +00:00
eva approved these changes 2026-11-02 16:03:24 +00:00
eva left a comment

GO on adce7b2.

The diff does what #41 asks. The render script stops calling make_paths_relative and resolves each external file against the extracted job context instead, so a Windows E:\ path that Blender can't make relative across drives now finds its copy in the upload. Files already inside the context keep their place, packed, generated and linked-library datablocks are left alone, and anything not in the context is named in the log with the one summary line. The runner's own output, home and sample_batches dirs come from one constant list, so the indexer and the Go side can't drift.

Checked:

  • One commit, title matches, base is the current develop tip (be76c82), CI check-and-test is green on this head.
  • Every placeholder gets filled, and the new Go test proves it. The context dir goes in through %q, which gives valid Python for Windows backslash paths too.
  • The Python resolver tests cover drive-letter paths, same-name files in different folders, case, //../ hops, UDIM tiles and the skipped dirs. CI fails if python3 is missing there, so they can't quietly skip.
  • Linux behaviour is the same for files that are already in the upload. The only change is that a host path outside the upload now gets pointed at the upload copy when there is one.

One note, not blocking: a file that isn't in the upload but does exist on the host still keeps its absolute path, so it can still read the host file. It now gets a warning by name, which is enough for this fix. Revert is a single-commit revert.

Not security-sensitive (no auth, secrets, network, deps or CI changes), so Linus can merge. After it lands, Jerry reruns the job on the ai1 Windows runner as the PR says.

GO on adce7b2. The diff does what #41 asks. The render script stops calling `make_paths_relative` and resolves each external file against the extracted job context instead, so a Windows `E:\` path that Blender can't make relative across drives now finds its copy in the upload. Files already inside the context keep their place, packed, generated and linked-library datablocks are left alone, and anything not in the context is named in the log with the one summary line. The runner's own `output`, `home` and `sample_batches` dirs come from one constant list, so the indexer and the Go side can't drift. Checked: - One commit, title matches, base is the current develop tip (be76c82), CI `check-and-test` is green on this head. - Every placeholder gets filled, and the new Go test proves it. The context dir goes in through `%q`, which gives valid Python for Windows backslash paths too. - The Python resolver tests cover drive-letter paths, same-name files in different folders, case, `//../` hops, UDIM tiles and the skipped dirs. CI fails if python3 is missing there, so they can't quietly skip. - Linux behaviour is the same for files that are already in the upload. The only change is that a host path outside the upload now gets pointed at the upload copy when there is one. One note, not blocking: a file that isn't in the upload but does exist on the host still keeps its absolute path, so it can still read the host file. It now gets a warning by name, which is enough for this fix. Revert is a single-commit revert. Not security-sensitive (no auth, secrets, network, deps or CI changes), so Linus can merge. After it lands, Jerry reruns the job on the ai1 Windows runner as the PR says.
blake approved these changes 2026-11-02 16:06:36 +00:00
blake left a comment

Blake security review: APPROVE on head adce7b258e

Scope: pkg/scripts/scripts/relink_context.py plus how the template and render.go wire it in. I read it myself and ran the functions locally on hostile inputs.

Nothing in this PR adds a new way out of the job dir. A relink target can only come from build_context_index, and that index is built from os.walk(root) relpaths, so it never holds ... A path is left unchanged only when its normpath is already under the root. On untrusted strings, _components only picks the basename and ranks the matches, so it can't steer a target outside the index. Results of my local runs:

  • /etc/passwd, ../../etc/passwd, //../../etc/passwd, .., /, //, D:, empty and \0 all returned None, so they're not relinked.
  • C:..\..\etc\wood.png, \\srv\share\wood.png, //?/C:/x/wood.png, \\?\UNC\h\s\WOOD.PNG and a\0/wood.png all resolved only to the in-context tex/wood.png.
  • Skipped top-level dirs (home/) stay unreachable: x/../../home/secret.png returned None.
  • UDIM: the base is a single component, so ../<UDIM> just becomes <root>/<UDIM>.
  • _inside uses commonpath, which works on whole components: <root>2/x is False and <root>/../x after normpath is False. normcase is a no-op on Linux and lower-cases on Windows, which is the right way round.
  • Placeholders are filled with Go %q and the skip list is Go-quoted constants. A Go-quoted WorkDir is a valid Python string literal for normal paths, and the WorkDir is runner-controlled.

Existing gaps, not added by this PR. Both are filed as follow-ups:

  1. #43: an unmatched absolute host path is left unchanged, and Blender still reads it. This is the same as make_paths_relative before. Reachable: any job owner can point a texture or library at a file the runner user can read.
  2. #44: _inside and the index don't use realpath, so an in-context symlink pointing outside passes. I confirmed this locally (tex/link.png -> /etc/passwd gets indexed and relinked). Not reachable today: runner.downloadContext uses workspace.ExtractTar, which only writes TypeDir and TypeReg entries, so archive symlinks and hardlinks are dropped. It's defense in depth only.

GO for Linus on adce7b258e.

**Blake security review: APPROVE on head `adce7b258e`** Scope: `pkg/scripts/scripts/relink_context.py` plus how the template and `render.go` wire it in. I read it myself and ran the functions locally on hostile inputs. **Nothing in this PR adds a new way out of the job dir.** A relink target can only come from `build_context_index`, and that index is built from `os.walk(root)` relpaths, so it never holds `..`. A path is left unchanged only when its normpath is already under the root. On untrusted strings, `_components` only picks the basename and ranks the matches, so it can't steer a target outside the index. Results of my local runs: - `/etc/passwd`, `../../etc/passwd`, `//../../etc/passwd`, `..`, `/`, `//`, `D:`, empty and `\0` all returned None, so they're not relinked. - `C:..\..\etc\wood.png`, `\\srv\share\wood.png`, `//?/C:/x/wood.png`, `\\?\UNC\h\s\WOOD.PNG` and `a\0/wood.png` all resolved only to the in-context `tex/wood.png`. - Skipped top-level dirs (`home/`) stay unreachable: `x/../../home/secret.png` returned None. - UDIM: the base is a single component, so `../<UDIM>` just becomes `<root>/<UDIM>`. - `_inside` uses commonpath, which works on whole components: `<root>2/x` is False and `<root>/../x` after normpath is False. `normcase` is a no-op on Linux and lower-cases on Windows, which is the right way round. - Placeholders are filled with Go `%q` and the skip list is Go-quoted constants. A Go-quoted WorkDir is a valid Python string literal for normal paths, and the WorkDir is runner-controlled. **Existing gaps, not added by this PR. Both are filed as follow-ups:** 1. #43: an unmatched absolute host path is left unchanged, and Blender still reads it. This is the same as `make_paths_relative` before. **Reachable**: any job owner can point a texture or library at a file the runner user can read. 2. #44: `_inside` and the index don't use realpath, so an in-context symlink pointing outside passes. I confirmed this locally (`tex/link.png -> /etc/passwd` gets indexed and relinked). **Not reachable today**: `runner.downloadContext` uses `workspace.ExtractTar`, which only writes `TypeDir` and `TypeReg` entries, so archive symlinks and hardlinks are dropped. It's defense in depth only. GO for Linus on `adce7b258e`.
linus merged commit 249cf62466 into develop 2026-11-02 16:07:15 +00:00
linus deleted branch ash/windows-context-paths-41 2026-11-02 16:07:15 +00:00
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
3 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
s1d3sw1ped/jiggablend!42
No description provided.