[security] fix(islo): contain workdir paths under workspace

[Hinotoi-agent]

Summary

This PR hardens the Islo provider's workspace path boundary so repo-local configuration cannot steer delegated sync outside /workspace.

It specifically:

• rejects absolute islo.workdir values before Islo client setup or sandbox creation;
• rejects relative .. values that clean outside /workspace;
• keeps valid nested relative workdirs such as team/repo working as /workspace/team/repo;
• adds regression coverage for the path escape and documents the enforced boundary.

Security issues covered

Issue	Impact	Severity
Repo-controlled Islo workdir can escape /workspace	A repo-local config can direct Islo workspace preparation, deletion, and archive extraction at broad sandbox paths such as /etc	Medium

Before this PR

• Crabbox loaded repo-local crabbox.yaml / .crabbox.yaml and applied islo.workdir directly.
• isloWorkspacePath() accepted absolute paths such as /etc.
• Relative paths such as ../etc were joined with /workspace and cleaned to /etc.
• Islo sync preparation then used that resolved path in rm -rf <workspace> && mkdir -p <workspace> when sync.delete was enabled.
• The behavior was not covered by a regression test.

After this PR

• islo.workdir is validated as a relative directory under /workspace.
• Absolute paths are rejected.
• Cleaned paths that resolve to /workspace itself or outside /workspace/ are rejected.
• Run() validates the path before Islo client setup or sandbox creation, so invalid config fails before provider-side side effects.
• The Islo tests now cover safe defaults, safe nested relative paths, absolute paths, dot/root-equivalent values, and .. escapes.

Why this matters

Islo delegated sync prepares the remote sandbox workdir before the user's command runs. A repository can carry Crabbox config, so the workdir value should not be able to redirect Crabbox's setup/deletion step at broad provider-side filesystem locations.

Without this guardrail, a malicious or compromised repository could include config such as:

provider: islo
islo:
  workdir: ../etc

On vulnerable code, that resolves to /etc and reaches the workspace preparation path.

How this differs from related E2B workdir fixes

This is a same-family variant of the E2B workspace hardening already merged in:

• fix: reject unsafe E2B workspaces #50
• fix: validate E2B workdir before create #55

Those PRs hardened E2B-specific workspace handling. This PR addresses the remaining Islo-specific implementation, which has a separate isloWorkspacePath() helper and delegated archive-sync flow.

The failure mode is similar, but the vulnerable code path is different:

• E2B fix: internal/providers/e2b/...
• This fix: internal/providers/islo/...

Attack flow

repo-local .crabbox.yaml / crabbox.yaml
    -> islo.workdir: ../etc
        -> isloWorkspacePath() cleans /workspace/../etc to /etc
            -> prepareWorkspace() builds rm -rf '/etc' && mkdir -p '/etc'
                -> delegated sync can delete or overwrite broad sandbox paths

Affected code

Issue	Files
Islo workdir escape	internal/providers/islo/sync.go, internal/providers/islo/backend.go, internal/providers/islo/backend_test.go
Documentation boundary	docs/features/islo.md, docs/providers/islo.md

Root cause

Issue: Islo workdir escape

• The Islo provider treated islo.workdir as a path component but did not enforce containment under /workspace.
• path.Join("/workspace", workdir) was used without checking whether the cleaned result stayed under the intended root.
• Absolute paths were accepted outright.
• The same resolved path was later used by workspace preparation, upload, extraction, and command execution.

CVSS assessment

Issue	CVSS v3.1	Vector
Islo workdir escape	7.1 Medium	CVSS:3.1/AV:L/AC:L/PR:N/UI:R/S:U/C:N/I:H/A:H

Rationale:

• Exploitation requires a user/operator to run Crabbox from a repository containing malicious Crabbox config.
• The impact is bounded to the delegated Islo sandbox environment, but can affect integrity and availability of broad sandbox paths before the intended command runs.

Safe reproduction steps

1. On vulnerable code, consider this repo-local config:

   provider: islo
   islo:
     workdir: ../etc
   sync:
     delete: true

2. The previous path resolver would compute:

   /workspace/../etc -> /etc
   

3. The workspace preparation command would then target the escaped path:

   rm -rf '/etc' && mkdir -p '/etc'

4. This PR's regression coverage exercises the same condition safely in unit tests, without running destructive commands against a real sandbox.

Expected vulnerable behavior

Before this PR:

• islo.workdir: ../etc resolved to /etc.
• islo.workdir: /etc was accepted directly.
• prepareWorkspace() used the escaped path in the generated remote shell command.

After this PR:

• both absolute values and cleaned escapes return a validation error before provider-side setup.

Changes in this PR

• Changes isloWorkspacePath() to return (string, error).
• Rejects absolute Islo workdir values.
• Rejects cleaned paths that resolve to /workspace itself or outside /workspace/.
• Propagates validation errors through both Run() and syncWorkspace().
• Validates Run() early, before Islo client setup or sandbox creation.
• Adds regression tests for safe values and unsafe absolute/escape values.
• Updates Islo documentation to state that workdirs are relative under /workspace.

Files changed

Category	Files	What changed
Islo provider	internal/providers/islo/sync.go	Added workspace containment validation and error return
Islo run flow	internal/providers/islo/backend.go	Validates workdir before provider setup and propagates validation errors
Tests	internal/providers/islo/backend_test.go	Added safe-path and escape-path regression coverage
Docs	docs/features/islo.md, docs/providers/islo.md	Documented the relative-under-/workspace boundary

Maintainer impact

• The patch is limited to the Islo provider and Islo docs.
• Valid relative workdirs continue to work.
• Nested relative workdirs such as team/repo remain supported.
• Absolute Islo workdirs now fail fast with a clear validation error.
• E2B and other providers are not changed.

Fix rationale

The Islo docs already describe sync into /workspace/<islo.workdir>, and the CLI flag help describes the value as an Islo sandbox working directory under /workspace. Enforcing that boundary in code makes the documented model true and prevents repo-local config from controlling broad filesystem deletion or extraction targets.

The containment check is intentionally small and local to the Islo path helper, so future call sites get the same validation automatically.

Type of change

• Security fix
• Tests
• Documentation update
• Refactor with no behavior change

Test plan

• go test ./internal/providers/islo ./internal/cli ./internal/providers/e2b -count=1
• go test ./... -count=1
• git diff --check
• node scripts/build-docs-site.mjs
• Independent pre-commit review of the final diff

Executed with:

gofmt -w internal/providers/islo/backend.go internal/providers/islo/backend_test.go internal/providers/islo/sync.go
go test ./internal/providers/islo ./internal/cli ./internal/providers/e2b -count=1
go test ./... -count=1
git diff --check
node scripts/build-docs-site.mjs

Token usage

• discovery tokens: partial/unknown; delegated discovery agents were used, but full-session accounting is not available in one reliable total
• validation tokens: partial/unknown
• duplicate-check tokens: partial/unknown
• PR/writeup tokens: partial/unknown
• total tokens: partial/unknown
• notes: exact local validation commands are listed above; no real secrets or credentials are included in this PR body

Disclosure notes

• This PR is intentionally bounded to the Islo delegated-sync workdir containment issue.
• It does not claim that command execution inside a deliberately chosen sandbox workdir is unexpected.
• The issue is the missing containment boundary before Crabbox prepares/deletes/uploads the workdir.
• No unrelated provider behavior is changed.