fix(orchestrator): preserve shared artifact permissions - #35
Conversation
Create atomic replacement files inside the engagement directory, preserve group-writable modes for host/container sharing, and mark generated workspaces setgid. Add regressions for artifact and workspace permissions.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a5925a036e
ℹ️ 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".
| if workspace_root.exists(): | ||
| shutil.rmtree(workspace_root, ignore_errors=True) | ||
| workspace_root.mkdir(parents=True, exist_ok=True) | ||
| workspace_root.chmod(0o2775) |
There was a problem hiding this comment.
Apply group-write defaults to generated workspace children
Setting only the workspace's setgid bit makes new children inherit its GID, but it does not make them group-writable. The container entrypoint leaves the default 022 umask in place, and engagement initialization creates scope.json and surfaces.jsonl with ordinary redirections, so these initial artifacts remain 0644 root:<shared-group>. The host-side heartbeat can then fail when normalize_active_scope() rewrites scope.json or appends process-log surface candidates before one of the newly changed atomic-replacement scripts happens to replace that file. Configure a shared umask/default ACL or explicitly initialize all container-created public artifacts as group-writable rather than relying on setgid alone.
Useful? React with 👍 / 👎.
Summary
/tmp0664modes and parent group inheritance for UI-readable artifacts2775) so container and host orchestrator share the same groupRoot cause
The runtime container writes engagement artifacts as root while the host orchestrator runs as a non-root user.
mktempcreated0600 root:rootfiles under/tmp; moving them onto the bind mount preserved those permissions. Listing a run then failed withPermissionErrorwhile reading or updatingscope.jsonandsurfaces.jsonl.Test plan
/projects/2/runsAPI returns 200 after the hotfix