fix: run tool terminal sessions as container user instead of root
- Remove compose-level user: 0:0 override from manifest_compiler.py so the entrypoint can start as root, fix mount ownership, and drop privileges to the container user internally. - Add get_manifest_container_user() helper to resolve the manifest-declared container user (with uid:gid fallback). - Pass container user through TerminalSession, TerminalManager, and the terminal WebSocket handler so docker exec is invoked with --user <user>. - Update and add unit tests for the manifest compiler and terminal session. - Record the additional root-user fix in the fix-pi-container-mount-permissions OpenSpec change/tasks. Quality gates: pytest tests/unit/ (226 passed), pytest tests/services/test_terminal_manager_multi.py (7 passed), ruff check on changed files (clean), mypy on changed files (clean)
This commit is contained in:
Developer
@@ -15,11 +15,19 @@ After implementing configurable tool container home directories, new `pi-agent`
|
||||
|
||||
## Fix
|
||||
|
||||
1. Add an Alembic data migration that updates the built-in `pi-agent` manifest:
|
||||
1. Remove the compose-level `user: 0:0` override from `manifest_compiler.py`. The
|
||||
Dockerfile intentionally omits `USER` so the entrypoint can start as root,
|
||||
fix mount ownership, and drop privileges to the container user internally.
|
||||
Pinning `user: 0:0` in the compose service forces `docker exec` sessions to
|
||||
run as root even after the entrypoint drops privileges.
|
||||
2. Pass the manifest-declared container user into terminal sessions so
|
||||
`docker exec` is invoked with `--user <user>`. This makes WebSocket terminal
|
||||
sessions run as the same non-root user as the main container process.
|
||||
3. Add an Alembic data migration that updates the built-in `pi-agent` manifest:
|
||||
- Change the repo mount target to `~/{{WORKSPACE_NAME}}`.
|
||||
- Keep `runtime.working_dir` as `/workspace` (the compatibility symlink).
|
||||
- Update the startup script to chown the real mount path (`$HOME/$WORKSPACE_NAME`).
|
||||
2. Update `manifest_compiler.py`:
|
||||
4. Update `manifest_compiler.py`:
|
||||
- Substitute `{{WORKSPACE_NAME}}` in mount targets in `compile_compose`.
|
||||
- Pass `WORKSPACE_NAME` as a container environment variable.
|
||||
- Generate the entrypoint symlink from the runtime `WORKSPACE_NAME` environment variable.
|
||||
@@ -28,25 +36,30 @@ After implementing configurable tool container home directories, new `pi-agent`
|
||||
- Start the container as root and drop privileges to the container user inside the entrypoint via `su`.
|
||||
- Do not create mount target directories or the `/workspace` symlink in the image when they depend on the runtime `{{WORKSPACE_NAME}}` placeholder.
|
||||
- Remove any stale literal `{{WORKSPACE_NAME}}` directory left over from older images at container startup.
|
||||
3. Update `instance_service.py` to pass `REPO_NAME` and `WORKSPACE_NAME` into manifest compilation.
|
||||
4. Remove the explicit repo mount from the built-in `pi-agent` manifest so the repo mount is synthesized by `compile_compose` rather than depending on tool config. Add a follow-up Alembic data migration that strips the `source_type: repo` mount from the manifest.
|
||||
5. Add `_get_repository_mount_name()` helper. When the instance is bound to a workspace, the helper returns the basename of `workspace.path`. For legacy repo-only instances it falls back to parsing the remote URL like `git clone` would, then to the user-provided repository name.
|
||||
6. Switch workspace storage layout to `/data/working-copies/{workspace_id}/{repo_name}/` so `git clone` creates the repo-named directory naturally, making `workspace.path.basename` the correct container mount name. This replaces the previous `/data/working-copies/{repo_id}/{workspace_name}/` layout.
|
||||
7. Update unit tests for the new behavior.
|
||||
5. Update `instance_service.py` to pass `REPO_NAME` and `WORKSPACE_NAME` into manifest compilation.
|
||||
6. Remove the explicit repo mount from the built-in `pi-agent` manifest so the repo mount is synthesized by `compile_compose` rather than depending on tool config. Add a follow-up Alembic data migration that strips the `source_type: repo` mount from the manifest.
|
||||
7. Add `_get_repository_mount_name()` helper. When the instance is bound to a workspace, the helper returns the basename of `workspace.path`. For legacy repo-only instances it falls back to parsing the remote URL like `git clone` would, then to the user-provided repository name.
|
||||
8. Switch workspace storage layout to `/data/working-copies/{workspace_id}/{repo_name}/` so `git clone` creates the repo-named directory naturally, making `workspace.path.basename` the correct container mount name. This replaces the previous `/data/working-copies/{repo_id}/{workspace_name}/` layout.
|
||||
9. Update unit tests for the new behavior.
|
||||
|
||||
## Affected files
|
||||
|
||||
- `apps/api/alembic/versions/2026_06_14_182955_fix_pi_agent_home_directory_mount.py`
|
||||
- `apps/api/alembic/versions/2026_06_15_090500_remove_pi_agent_explicit_repo_mount.py`
|
||||
- `apps/api/src/services/build/manifest_compiler.py`
|
||||
- `apps/api/src/services/terminal/terminal_session.py`
|
||||
- `apps/api/src/services/terminal/terminal_manager.py`
|
||||
- `apps/api/src/api/system/terminal.py`
|
||||
- `apps/api/src/services/shared/workspace_manager.py`
|
||||
- `apps/api/src/services/tool/instance_service.py`
|
||||
- `apps/api/tests/unit/test_manifest_compiler.py`
|
||||
- `apps/api/tests/unit/test_terminal_session.py`
|
||||
- `apps/api/tests/unit/test_instance_service.py`
|
||||
- `apps/api/tests/unit/test_alembic_migrations.py`
|
||||
|
||||
## Verification
|
||||
|
||||
- `pytest apps/api/tests/unit/test_manifest_compiler.py`
|
||||
- `pytest apps/api/tests/unit/test_terminal_session.py`
|
||||
- `pytest apps/api/tests/unit/test_alembic_migrations.py`
|
||||
- `ruff`, `mypy`, `npm run typecheck`, `npm run lint`
|
||||
- `ruff`, `mypy`, `npm run typecheck`, `npm run lint`
|
||||
|
||||
@@ -7,6 +7,10 @@
|
||||
- [x] Remove explicit repo mount from pi-agent manifest; synthesize mount in compile_compose
|
||||
- [x] Add _get_repository_mount_name() helper to derive workspace name from remote URL
|
||||
- [x] Switch workspace storage layout to /data/working-copies/{workspace_id}/{repo_name}/
|
||||
- [x] Update unit tests
|
||||
- [x] Run quality gates (pytest unit, ruff, mypy)
|
||||
- [x] Update unit tests for workspace/home-directory migration
|
||||
- [x] Run quality gates for workspace/home-directory migration
|
||||
- [x] Remove compose-level `user: 0:0` override so entrypoint can drop privileges
|
||||
- [x] Pass manifest-declared container user to terminal sessions via `docker exec --user`
|
||||
- [x] Update unit tests for container user/terminal changes
|
||||
- [ ] Run quality gates for container user/terminal changes
|
||||
- [ ] Commit and push
|
||||
|
||||
Reference in New Issue
Block a user