fix(sandbox): 产物目录 bootstrap 降权,堵住 chown 跟随符号链接的容器内提权

会话级容器让「产物目录 bootstrap 以 root 执行」变成一条可复现的提权链:
沙箱账号拥有 /workspace,可以把产物目录换成指向 /etc 的符号链接;下一次
执行时 root 跑的 chown/chmod 会跟随链接,把 /etc 的属主交给沙箱账号,随后
删除重建 passwd 即可让该账号在下一次 exec 时解析成 uid 0——本 PR 刚做的
shell_exec 降权和文件操作降权会一并失效。一次性 docker run --rm 下两次执行
不共享文件系统,这条链走不通。真机验证过完整链路。

修法是让 bootstrap 和其它 exec 一样以沙箱账号运行:chown 对自己刚建的目录
是 no-op,对不属于自己的目标被内核拒绝。同时把 dockerExecUser 的空值兜底
从 root 改成 DefaultSandboxExecUser,漏传账号只会失去权限而非拿到权限,
也让三个后端对「空 User」的语义一致。

顺带订正若干过强的声明:find 只对路径最后一段不跟随符号链接,中间层的链接
仍由内核展开,因此 read_sandbox_file 的「拒绝符号链接」只覆盖路径本身指向
链接的情况;对应单测原先 mock 了一个真实 find 不会产生的返回值,改为断言
它实际覆盖的场景。文件头注释仍写着文件操作走 archive 接口,也一并更新。
This commit is contained in:
wizardchen
2026-08-24 12:09:13 +08:00
committed by lyingbug
parent d6ddad5430
commit adcb9fc10d
8 changed files with 200 additions and 54 deletions
+20 -6
View File
@@ -74,11 +74,19 @@ ENTRYPOINT 拼到 Cmd 前面,所以只设 Cmd 时,任何声明了 ENTRYPOINT
执行命令的东西——包括普通技能脚本,不需要 root——都可以起一个后台循环持续 `touch` 标记,
把自己维持成「一直活跃」。当前没有硬寿命上限,需要的话应由部署方在 daemon 侧限制。
**所有 exec 都以 `user`(uid 1000) 运行。** 脚本执行、`shell_exec`以及全部文件操作都显式
`DefaultSandboxExecUser`。唯一的例外是 manager 自己的 bootstrap:它要把产物目录 `chown`
**给**沙箱账号,因此不能以该账号执行,这也是 `RemoteExecRequest.User` 留空的唯一用途
**所有 exec 都以 `user`(uid 1000) 运行,没有例外** 脚本执行、`shell_exec`、全部文件操作
以及 manager 自己的产物目录 bootstrap 都跑在沙箱账号下;`RemoteExecRequest.User` 留空时
适配器解析成 `DefaultSandboxExecUser` 而不是 root,漏传账号只会失去权限、不会拿到权限
bootstrap 尤其不能以 root 跑:产物目录位于会话自己可写的 `/workspace` 下,而 `chown`/`chmod`
会跟随符号链接。会话只要把产物目录换成指向 `/etc` 的链接,一次 root bootstrap 就会把 `/etc`
的属主交给沙箱账号,接着改写 `passwd` 即可让该账号在下一次 exec 时变成 uid 0(真机验证过)。
以沙箱账号执行时这条链直接断在内核:`chown` 对不属于自己的目标一律失败。
容器 `CapDrop: ALL` 之后额外补回 CHOWN/DAC_OVERRIDE/FOWNER/FSETID/SETGID/SETUID/KILL
这是 root 装包和修属主需要的最小集合;Docker 默认给的 NET_RAW、MKNOD、SYS_CHROOT 等一律不给。
Docker 默认给的 NET_RAW、MKNOD、SYS_CHROOT 等一律不给。注意这批 capability 是给容器内
**root** 用的(装包、修属主),而目前没有任何 exec 以 root 运行,因此它们对现有路径是冗余的;
保留是为了自定义镜像里用 `sudo` 装包的场景,收紧它们是可以独立推进的加固项。
**文件操作走 exec,不走 archive 接口。** archive 接口(`PUT`/`GET`/`HEAD /archive`)由 daemon
执行,这意味着两件事同时成立:它忽略 exec user 一律以 root 操作,并且会在路径解析时跟随符号
@@ -86,8 +94,14 @@ ENTRYPOINT 拼到 Cmd 前面,所以只设 Cmd 时,任何声明了 ENTRYPOINT
`ln -s /root /workspace/output/esc` 之后,`/workspace/output/esc/secret.txt` 既能通过守卫,
又会被 daemon 以 root 读出来(真机验证过,不是推演)。改成以沙箱账号 exec 之后,能不能读写
由内核判定,符号链接指向哪里都不再重要,也不存在「先校验后使用」之间被换掉链接的窗口。
`Stat``find`,它不跟随符号链接,因此链接会如实报告为 `other` 类型,要求正规文件的调用方
在尝试读取之前就会拒绝。archive 接口里只剩 `HEAD` 还在用,且仅用于读固定路径的活跃标记。
`Stat``find`,它不跟随**最后一段**路径,因此路径本身是链接会如实报告为 `other` 类型,
要求正规文件的调用方在尝试读取之前就会拒绝。
这个保证到最后一段为止:中间层的链接由内核在路径解析时展开,`/workspace/output/链接/passwd`
仍会 stat 成普通文件(真机验证过)。因此「只读产物目录」是一个约定而非权限边界——绕过它读到的
东西,沙箱账号本来就能用 `shell_exec` 读到,真正的边界始终是内核的权限检查。
archive 接口里只剩 `HEAD` 还在用,且仅用于读固定路径的活跃标记。
**PID 1 开 tini`HostConfig.Init`)。** 容器入口是 `sleep`,它从不调用 `wait()`。长会话里
后台进程一旦活得比启动它的 exec 久,退出后就会变成没人回收的僵尸,堆到 `pids_limit` 之后所有
+8 -2
View File
@@ -190,8 +190,14 @@ func (t *ReadSandboxFileTool) Execute(ctx context.Context, args json.RawMessage)
}
// The directory guard above is a string prefix test, so it cannot tell that
// a symlink under the output directory points somewhere else entirely. The
// backends report a link as-is rather than as its target, which makes this
// the check that keeps such a path from being read at all.
// backends stat the final component without following it, so this refuses a
// path that names a link.
//
// A link in the MIDDLE of the path is still resolved by the kernel and is
// not caught here. That leaves the artifact-directory convention evadable,
// but not the privilege boundary: the read runs as the sandbox account, so
// it can only return what that account could already have read via
// shell_exec.
if stat.Type != sandbox.RemoteEntryFile {
return &types.ToolResult{
Success: false,
+9 -4
View File
@@ -81,9 +81,14 @@ func TestReadSandboxFileReturnsSmallTextOnlyInOutput(t *testing.T) {
}
// The output-directory guard is a string prefix test, so a symlink planted
// under that directory satisfies it while pointing anywhere. The backends
// report a link as-is instead of following it, and this is the check that
// turns that into a refusal before any read is attempted.
// under that directory satisfies it while pointing anywhere. The backends stat
// the final component without following it, and this is the check that turns
// that into a refusal before any read is attempted.
//
// The path here names the link itself, which is the case this actually covers.
// A link used as an intermediate component is resolved by the kernel and still
// stats as a regular file; see the note in Execute for why that is a convention
// leak rather than a privilege one.
func TestReadSandboxFileRefusesNonRegularFile(t *testing.T) {
source := &fakeSandboxFileSource{
stat: &sandbox.RemoteStatEntry{
@@ -96,7 +101,7 @@ func TestReadSandboxFileRefusesNonRegularFile(t *testing.T) {
result, err := NewReadSandboxFileTool(source).Execute(
sandboxFileTestContext(),
json.RawMessage(`{"path":"/workspace/output/esc/secret.txt"}`),
json.RawMessage(`{"path":"/workspace/output/esc"}`),
)
require.NoError(t, err)
+95 -5
View File
@@ -321,11 +321,10 @@ func TestDockerBackendScriptExecutionRefreshesActivityMarkerIntegration(t *testi
}
before := dockerActivityMarkerMTime(t, ctx, summaries[0].ID)
// Exec directly as the unprivileged account, with no root-run step in
// between: WeKnora happens to prepare the artifact directory as root
// before each script today, and leaning on that would let the marker
// silently stop tracking the account that actually runs user code.
// The marker has one-second resolution on most filesystems.
// Exec directly as the unprivileged account: this asserts that the account
// running user code refreshes the marker itself, rather than relying on
// some other step happening to touch it first. The marker has one-second
// resolution on most filesystems.
time.Sleep(2 * time.Second)
result, err := client.Exec(ctx, handle, RemoteExecRequest{
Command: "echo",
@@ -427,6 +426,97 @@ with open(%q) as handle:
}
}
// A session owns /workspace, so it can replace its own artifact directory with
// a symlink pointing anywhere in the container. chown and chmod follow
// symlinks, so if the pre-execution bootstrap ran as root it would hand the
// session ownership of the link's target — /etc here, which is enough to
// rewrite passwd and give the sandbox account uid 0 on the next exec.
//
// Session-scoped containers are what make this reachable: the link planted by
// one execution is still there when the next one runs the bootstrap. Under the
// old one-shot `docker run --rm` the two never shared a filesystem.
func TestDockerBackendArtifactBootstrapDoesNotFollowSymlinkIntegration(t *testing.T) {
cfg := dockerIntegrationConfig(t)
manager := newDockerIntegrationManager(t, cfg)
ctx, cancel := context.WithTimeout(
types.WithSandboxTenantID(context.Background(), dockerIntegrationTenantID),
5*time.Minute,
)
defer cancel()
sessionID := fmt.Sprintf("docker-chown-escape-%d", time.Now().UnixNano())
t.Cleanup(func() {
cleanupCtx, cleanupCancel := context.WithTimeout(
types.WithSandboxTenantID(context.Background(), dockerIntegrationTenantID),
time.Minute,
)
defer cleanupCancel()
_ = manager.DestroySession(cleanupCtx, sessionID)
})
if first := runDockerScript(t, ctx, manager, sessionID, `print('seed')`); !first.IsSuccess() {
t.Fatalf("seed execution failed: %#v", first)
}
client, err := NewDockerRemoteClient(cfg)
if err != nil {
t.Fatalf("build docker client: %v", err)
}
summaries, err := client.List(ctx, RemoteListFilter{
Metadata: map[string]string{remoteMetadataSessionID: sessionID},
})
if err != nil || len(summaries) != 1 {
t.Fatalf("expected exactly one container for the session: %v %#v", err, summaries)
}
handle, err := client.Connect(ctx, summaries[0].ID)
if err != nil {
t.Fatalf("Connect: %v", err)
}
ownerOfEtc := func() string {
t.Helper()
result, err := client.Exec(ctx, handle, RemoteExecRequest{
Command: "stat",
Args: []string{"-c", "%U:%G", "/etc"},
User: DefaultSandboxExecUser,
Timeout: 30 * time.Second,
})
if err != nil || result.ExitCode != 0 {
t.Fatalf("stat /etc failed: %v %#v", err, result)
}
return strings.TrimSpace(result.Stdout)
}
before := ownerOfEtc()
if before != "root:root" {
t.Fatalf("/etc should start out root-owned, got %q", before)
}
// Exactly what a model with shell_exec can do: it owns /workspace.
plant, err := client.Exec(ctx, handle, RemoteExecRequest{
Shell: true,
Command: fmt.Sprintf(
"rm -rf %s && ln -s /etc %s", SessionOutputRoot, SessionOutputRoot,
),
User: DefaultSandboxExecUser,
Timeout: 30 * time.Second,
})
if err != nil || plant.ExitCode != 0 {
t.Fatalf("planting the symlink failed: %v %#v", err, plant)
}
// Any further execution runs the bootstrap against the planted path.
if second := runDockerScript(t, ctx, manager, sessionID, `print('after')`); second == nil {
t.Fatal("second execution returned no result")
}
if after := ownerOfEtc(); after != "root:root" {
t.Fatalf("the artifact bootstrap chowned through the symlink: /etc is now %q, want root:root",
after)
}
}
// stopDockerContainerForTest stops a container behind the adapter's back, the
// way a host reboot or an operator with a shell would.
func stopDockerContainerForTest(ctx context.Context, id string) error {
+36 -17
View File
@@ -14,11 +14,16 @@
// Get/List → GET /containers/json?filters=label=…
// Delete → DELETE /containers/{id}?force=1
// Exec → POST /containers/{id}/exec → /exec/{id}/start (hijack)
// Files → PUT/GET/HEAD /containers/{id}/archive
//
// Three operations have no Engine API and are implemented as exec:
// MakeDir, Remove and ListDir. ListDir uses `find -printf`, which needs GNU
// findutils in the image; the standard WeKnora sandbox image provides it.
// Every file operation — WriteFile, ReadFile, Stat, MakeDir, Remove, ListDir —
// is an exec running as the sandbox account, NOT a call to /archive. The
// archive endpoints run as root and resolve symlinks, so a session that plants
// a link inside its own workspace could read or overwrite anything in the
// container through them. Going through exec puts the kernel back in charge of
// who may touch what. Do not "simplify" these back onto /archive.
//
// ListDir and Stat use `find -printf`, which needs GNU findutils in the image;
// the standard WeKnora sandbox image provides it.
//
// Two Docker facts shape the rest of this file:
//
@@ -666,18 +671,24 @@ func dockerExecCommand(req RemoteExecRequest, timeout time.Duration) []string {
// dockerExecUser resolves which account a command runs as.
//
// An empty user is reserved for the manager's own bootstrap, which chowns the
// artifact directory TO the sandbox account and therefore cannot run as it. It
// is not a general default: the envd-backed backends resolve a blank user to
// DefaultSandboxExecUser (E2B authenticates the data plane as that account,
// Cube hands the field to envd, which defaults the same way), so a path that
// left this to the adapter would run as root here and unprivileged there.
// Every caller-reachable path names the account explicitly.
// A blank user resolves to the sandbox account. It must never resolve to root:
// this function is the single choke point for every exec the daemon runs, so a
// caller that forgets to name an account has to lose privileges here, not
// silently gain them. It also makes the backends agree — E2B authenticates its
// data plane as DefaultSandboxExecUser and Cube hands a blank field to envd,
// which defaults the same way.
//
// This used to fall back to root for the manager's artifact-directory
// bootstrap. That was a container-escape primitive: chown follows symlinks, so
// a session that replaced its own artifact directory with a link to /etc got
// the root-run bootstrap to hand it ownership of /etc, and from there uid 0 by
// rewriting passwd. The bootstrap now names the account like everyone else and
// simply fails when it is aimed at something the account does not own.
func dockerExecUser(user string) string {
if strings.TrimSpace(user) == "" {
return "root"
if trimmed := strings.TrimSpace(user); trimmed != "" {
return trimmed
}
return user
return DefaultSandboxExecUser
}
// dockerExecWasKilled reports whether an exit code means the wrapper killed
@@ -776,9 +787,17 @@ func (c *DockerRemoteClient) ReadFile(
// Stat returns metadata for one path.
//
// find does not follow symlinks, so a link reports as RemoteEntryOther rather
// than as whatever it points at. Callers that only accept regular files
// therefore refuse it before any read is attempted.
// find reports the FINAL component without following it, so a path that names a
// link reports RemoteEntryOther rather than whatever it points at, and callers
// that only accept regular files refuse it before any read is attempted.
//
// That guarantee stops at the final component. Intermediate components are
// resolved by the kernel during path lookup, exactly as they are for any other
// process, so `/workspace/output/link-to-etc/passwd` stats as a regular file —
// verified against a real daemon. Reads through such a path are not a
// privilege boundary being crossed, only the "stay inside the artifact
// directory" convention: ReadFile still runs as the sandbox account, so it
// returns what that account could have read anyway with shell_exec.
func (c *DockerRemoteClient) Stat(
ctx context.Context,
handle RemoteSandboxHandle,
+11 -8
View File
@@ -550,14 +550,17 @@ func TestDockerClientExecWrapsCommandWithTimeoutAndActivityMarker(t *testing.T)
opts.Cmd[3:], "the command must reach the shell as positional args, never interpolated")
}
// A blank user is reserved for the manager's own privileged bootstrap. Every
// other adapter resolves it to the sandbox account, so a caller that forgets to
// name one here would silently gain root instead of losing privileges.
func TestDockerExecUserOnlyFallsBackToRootWhenUnnamed(t *testing.T) {
// Every exec the daemon runs passes through dockerExecUser, so a caller that
// forgets to name an account has to lose privileges here rather than gain them.
// Falling back to root used to be a container-escape primitive: the artifact
// bootstrap chowns a path inside the session's own workspace, and chown follows
// symlinks, so root + a planted link meant the session could take ownership of
// /etc and rewrite passwd to give itself uid 0.
func TestDockerExecUserNeverFallsBackToRoot(t *testing.T) {
require.Equal(t, DefaultSandboxExecUser, dockerExecUser(DefaultSandboxExecUser))
require.Equal(t, "1000:1000", dockerExecUser("1000:1000"))
require.Equal(t, "root", dockerExecUser(""))
require.Equal(t, "root", dockerExecUser(" "))
require.Equal(t, DefaultSandboxExecUser, dockerExecUser(""))
require.Equal(t, DefaultSandboxExecUser, dockerExecUser(" "))
}
// Cancelling an exec leaves the copy goroutine writing into the output buffers.
@@ -611,8 +614,8 @@ func TestDockerClientExecShellPassesCommandAsPositionalArgument(t *testing.T) {
require.NoError(t, err)
opts := engine.execOptions[0]
require.Equal(t, []string{"weknora-exec", `echo "a b"; rm -rf /nope`}, opts.Cmd[3:])
require.Equal(t, "root", opts.User,
"a blank user is the manager's bootstrap escape hatch; callers name the account")
require.Equal(t, DefaultSandboxExecUser, opts.User,
"an unnamed account must resolve to the sandbox user, never to root")
}
func TestDockerClientExecRejectsShellWithArgs(t *testing.T) {
+13 -5
View File
@@ -304,11 +304,18 @@ func (m *SessionBoundManager) Execute(ctx context.Context, cfg *ExecuteConfig) (
return m.ephemeral.ExecuteOnHandle(ctx, handle, cfg)
}
// ensureExecutionOutputDir creates the skill artifact directory and grants
// DefaultSandboxExecUser write access before script execution. envd MakeDir
// often leaves the path root-owned on Cube; the follow-up chown/chmod runs as
// root via envd (empty User). Best-effort: failures are logged and do not
// abort the upcoming script execution.
// ensureExecutionOutputDir creates the skill artifact directory and makes sure
// DefaultSandboxExecUser can write to it before script execution.
//
// This runs AS that account, never as root. The directory sits inside the
// session's own writable workspace, and chown/chmod follow symlinks, so a
// root-run bootstrap can be aimed at any directory in the container: a session
// that swaps its artifact directory for a link to /etc gets handed ownership of
// /etc, and from there uid 0 by rewriting passwd. Running as the sandbox
// account makes that a no-op — chown succeeds on the directory MakeDir just
// created for it and is refused by the kernel on anything else.
//
// Best-effort: failures are logged and do not abort the upcoming execution.
func (m *SessionBoundManager) ensureExecutionOutputDir(
ctx context.Context,
handle RemoteSandboxHandle,
@@ -334,6 +341,7 @@ func (m *SessionBoundManager) ensureExecutionOutputDir(
result, err := m.client.Exec(ctx, handle, RemoteExecRequest{
Shell: true,
Command: line,
User: execUser,
Timeout: sessionArtifactDirBootstrapTimeout,
})
if err != nil {
+8 -7
View File
@@ -89,16 +89,17 @@ func TestSessionBoundManagerExecuteEnsuresOutputDir(t *testing.T) {
require.True(t, execs[0].Shell)
require.Contains(t, execs[0].Command, "chown user:user")
require.Contains(t, execs[0].Command, SessionOutputRoot)
require.Empty(t, execs[0].User,
"the bootstrap chowns the directory TO the sandbox account, so it is the "+
"one exec that must keep the adapter's privileged default")
require.Equal(t, DefaultSandboxExecUser, execs[0].User,
"chown follows symlinks, so a root-run bootstrap can be aimed at /etc by "+
"a session that swaps its artifact directory for a link; running as the "+
"sandbox account is what makes that attempt fail")
}
// shell_exec carries a command line the model wrote, which makes it the exec
// path an injected prompt reaches most directly. The account it runs as must
// therefore be pinned here rather than left to each adapter: Docker resolves a
// blank user to root, while the envd-backed backends resolve it to the sandbox
// account, so omitting it would hand out different privileges per backend.
// path an injected prompt reaches most directly. The account it runs as is
// pinned here rather than left to each adapter, so that reading this call site
// answers "as whom does model-authored input run" without having to trust that
// all three adapters agree on what a blank user means.
func TestSessionBoundManagerShellExecRunsAsSandboxUser(t *testing.T) {
client := newFakeRemoteClient(SandboxTypeCube)
cfg := DefaultConfig()