runner: Render from the job context, not absolute host paths (#41) #42
Labels
No labels
Compat/Breaking
Kind/Bug
Kind/Documentation
Kind/Enhancement
Kind/Feature
Kind/Security
Kind/Testing
Priority
Critical
Priority
High
Priority
Low
Priority
Medium
Reviewed
Confirmed
Reviewed
Duplicate
Reviewed
Invalid
Reviewed
Won't Fix
Status
Abandoned
Status
Blocked
Status
Need More Info
blocked
draft
No milestone
No project
No assignees
3 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
s1d3sw1ped/jiggablend!42
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "ash/windows-context-paths-41"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
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 MThe runner's own
output,homeandsample_batchesdirs 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
-racein./pkg/scriptsand./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) andTestCreateRenderScript_FillsEveryPlaceholder.createRenderScriptwrites. The centre pixel was read from the EXR:E:\Lib\Runtime\Textures\tex.png(absent on host)relinked 1,not in context 0E:\Lib\Elsewhere\nothere.png(nowhere)Warning: Image 'seed.png' is not in the job context: E:/Lib/Elsewhere/nothere.pngThe Windows check on ai1 is still needed: once this merges, Jerry reruns the job there.
Fixes #41
GO on
adce7b2.The diff does what #41 asks. The render script stops calling
make_paths_relativeand resolves each external file against the extracted job context instead, so a WindowsE:\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 ownoutput,homeandsample_batchesdirs come from one constant list, so the indexer and the Go side can't drift.Checked:
be76c82), CIcheck-and-testis green on this head.%q, which gives valid Python for Windows backslash paths too.//../hops, UDIM tiles and the skipped dirs. CI fails if python3 is missing there, so they can't quietly skip.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 security review: APPROVE on head
adce7b258eScope:
pkg/scripts/scripts/relink_context.pyplus how the template andrender.gowire 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 fromos.walk(root)relpaths, so it never holds... A path is left unchanged only when its normpath is already under the root. On untrusted strings,_componentsonly 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\0all returned None, so they're not relinked.C:..\..\etc\wood.png,\\srv\share\wood.png,//?/C:/x/wood.png,\\?\UNC\h\s\WOOD.PNGanda\0/wood.pngall resolved only to the in-contexttex/wood.png.home/) stay unreachable:x/../../home/secret.pngreturned None.../<UDIM>just becomes<root>/<UDIM>._insideuses commonpath, which works on whole components:<root>2/xis False and<root>/../xafter normpath is False.normcaseis a no-op on Linux and lower-cases on Windows, which is the right way round.%qand 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:
make_paths_relativebefore. Reachable: any job owner can point a texture or library at a file the runner user can read._insideand the index don't use realpath, so an in-context symlink pointing outside passes. I confirmed this locally (tex/link.png -> /etc/passwdgets indexed and relinked). Not reachable today:runner.downloadContextusesworkspace.ExtractTar, which only writesTypeDirandTypeRegentries, so archive symlinks and hardlinks are dropped. It's defense in depth only.GO for Linus on
adce7b258e.