mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-10-06 23:49:41 +02:00
fix(docker): close the three adoption gaps the negative guarantee missed
Review follow-ups to #357. Each is a path that still touched, or still hid, a container Codeman does not own. **Export still mutated it.** The four fail-closed layers cover create/start/ stop/remove, but `POST /api/docker-cases/:name/export` reaches the container twice through neither: a full export `docker commit`s it, and even a workspace-only export `docker pause`s it first for snapshot consistency. Pause freezes the owner's processes for as long as the tar takes, on a container we promised not to touch. Full export is refused for an adopted case (it packages someone else's container, with their logins, into a bundle Codeman hands out); workspace-only keeps working and no longer pauses, accepting a live filesystem the way `tar` does on any running host directory. **A freshly linked OWNED case became unusable.** The run menu now probes the container for its CLIs, and a failed probe hides every agent mode behind the reason. For an adopted case that is right. For an owned one the container does not exist until the first session launches it, so every newly linked Docker case answered `container "codeman-case-x" not found (adoption never creates a container — start it yourself first)` and offered nothing but Shell, for a container the launch chain was about to create itself. A failed probe is recorded only when the case is adopted; `CaseInfo.docker.owned` is on the wire so the frontend can tell them apart. Verified in a browser: owned-with-no- container offers all ten modes and no notice, adopted-but-stopped offers Shell and says why. **Multi-user gating.** Adoption is admin-only, unlike `docker-link` beside it. Linking creates OUR container, whose sole bind mount `isWorkingDirAllowed` has already confined to the caller's space; an adopted container's mounts are whatever its owner gave it, so one mounting `/` hands the adopter a shell over the whole host — exactly the workspace scoping multi-user mode exists to enforce. Listing the engine's containers and browsing directories inside an arbitrary one are machine-level reads and follow the docker-HOST policy for the same reason. The preflight is deliberately not admin-only: the run menu fires it for every docker case, so it admits a non-admin for a container already linked to a case they can access, and nothing else. Verified end to end against a real pre-existing root container (alpine + tmux, no bind mounts): adopt, claude session inside it, workspace export, session close and case unlink all left `StartedAt`, `RestartCount`, `Pid` and `Paused` untouched; the pane ran the CONTAINER's claude, without `--dangerously-skip-permissions`; a stopped container was refused at both preflight and launch and was never started. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TecFD9hvPYJ1mkkMtBQbT1
This commit is contained in:
@@ -388,3 +388,66 @@ describe('adopted container: probe modes come from the CLI registry', () => {
|
||||
expect(getCli('shell')?.discovery.binaries[0]).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe('adopted container: export never touches the container', () => {
|
||||
const routes = readFileSync(new URL('../src/web/routes/case-routes.ts', import.meta.url), 'utf8');
|
||||
const exporter = readFileSync(new URL('../src/docker-export.ts', import.meta.url), 'utf8');
|
||||
|
||||
it('refuses a full-image export, which would commit a container we do not own', () => {
|
||||
expect(routes).toContain("if (mode === 'full' && dockerCase.owned === false)");
|
||||
});
|
||||
|
||||
it('never pauses an adopted container for the workspace tar', () => {
|
||||
// `docker pause` freezes the owner's processes for as long as the tar takes.
|
||||
// It is the one export step that touches the container at all.
|
||||
expect(exporter).toContain('!isAdoptedContainer(docker) && (await isContainerRunning(');
|
||||
});
|
||||
});
|
||||
|
||||
describe('adopted container: naming a foreign container is machine-level', () => {
|
||||
const routes = readFileSync(new URL('../src/web/routes/case-routes.ts', import.meta.url), 'utf8');
|
||||
const routeFor = (marker: string) => routes.slice(routes.indexOf(marker), routes.indexOf(marker) + 1400);
|
||||
|
||||
it('admin-gates adoption in multi-user mode, unlike docker-link', () => {
|
||||
// docker-link only ever creates OUR container, whose sole bind mount is a
|
||||
// workspace isWorkingDirAllowed already confined. An adopted container's
|
||||
// mounts belong to its owner — one mounting `/` hands the adopter the host.
|
||||
expect(routeFor("'/api/cases/docker-adopt'")).toContain('adminOnly(req, reply)');
|
||||
});
|
||||
|
||||
it('admin-gates enumerating and browsing containers', () => {
|
||||
expect(routeFor("'/api/docker-hosts/:hostId/containers'")).toContain('adminOnly(req, reply)');
|
||||
expect(routeFor("'/api/docker-cases/browse'")).toContain('adminOnly(req, reply)');
|
||||
});
|
||||
|
||||
it('lets a non-admin preflight only a container linked to a case they own', () => {
|
||||
// NOT plain adminOnly: the run menu probes this for every docker case to learn
|
||||
// which CLIs the container has, so an admin-only gate would hide every agent
|
||||
// mode from a non-admin's own docker case.
|
||||
const route = routeFor("'/api/docker-cases/adopt-preflight'");
|
||||
expect(route).toContain('if (!isAdmin(req))');
|
||||
expect(route).toContain('canAccessOwned(getAuthUser(req), item.owner)');
|
||||
expect(route).not.toContain('adminOnly(req, reply)');
|
||||
});
|
||||
});
|
||||
|
||||
describe('adopted container: a missing container means different things per ownership', () => {
|
||||
const ui = readFileSync(new URL('../src/web/public/session-ui.js', import.meta.url), 'utf8');
|
||||
const routes = readFileSync(new URL('../src/web/routes/case-routes.ts', import.meta.url), 'utf8');
|
||||
|
||||
it('records a probe failure only for an adopted case', () => {
|
||||
// An OWNED container does not exist until the first session launches it, so
|
||||
// "not found" is the expected answer for every freshly linked Docker case.
|
||||
// Treating it as a fault hid every agent mode behind an error telling the user
|
||||
// to start a container the launch chain was about to create itself.
|
||||
const probe = ui.slice(ui.indexOf('async _probeDockerCaseModes('), ui.indexOf('async _loadRunModeHistory('));
|
||||
expect(probe).toContain('if (activeCase?.docker?.owned === false) {');
|
||||
expect(probe.indexOf('if (activeCase?.docker?.owned === false) {')).toBeLessThan(
|
||||
probe.indexOf('this._dockerCaseProbeError[name] =')
|
||||
);
|
||||
});
|
||||
|
||||
it('ships the ownership flag the UI reads that decision from', () => {
|
||||
expect(routes).toContain('...(dockerCase.owned === false ? { owned: false } : {}),');
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user