feat(sdk/go): add user-scoped filesystem views

[wangxuw]

Add Files.ForUser as an immutable identity scope and propagate the selected user through E2B-compatible HTTP file requests and filesystem RPC authorization headers. Preserve unscoped request behavior and existing access tokens.

Refs #1346

Autonomously-by: Codex:GPT-5

[cubesandboxbot]

req.SetBasicAuth unconditionally overwrites any existing Authorization header. Nothing sets it before this point today (newEnvdRequest only adds X-Access-Token + traffic tokens), so this isn't a live bug — but it's fragile and duplicates the existing basicAuthUser helper, which builds exactly this value. Consider req.Header.Set("Authorization", basicAuthUser(opts.user)) inside the guard instead.

Also worth a comment: this guard is deliberate, but it makes the wire format asymmetric with startProcess (which sends Basic root: for an empty user via basicAuthUser("") → defaultEnvdUser). Here, unscoped requests emit no Authorization header while ForUser("root") emits Basic root:. That preserves legacy unscoped behavior, but it's subtle enough to warrant a one-line note so a future reader doesn't "fix" the guard.

[cubesandboxbot]

This switches the only octet-stream→multipart fallback test to the user-scoped path, so the unscoped Files().Write fallback — and its request shape (no username param) — is no longer exercised. TestFilesWithoutUserPreservesUnscopedRequestShape covers only Read and List. Since the PR's stated goal is to preserve unscoped request behavior, it'd be worth keeping (or adding) an unscoped Write assertion that the fallback still works with no username in the query on both attempts.

[cubesandboxbot]

Review of PR #1348 — feat(sdk/go): add user-scoped filesystem views

Overall: This is a clean, well-scoped addition. Files.ForUser(user) returns a copy of the filesystem view that carries an immutable identity, threaded through both filesystem transports — the E2B-compatible HTTP GET/POST /files endpoint (via the username query parameter) and the filesystem Connect RPC endpoints ListDir/Stat/Remove/Move/MakeDir/WatchDir (via Basic auth). The unscoped request shape and existing X-Access-Token/traffic tokens are preserved, and the change is backward-compatible at the public API level (only the unexported fileReader/fileWriter/fileFiler interfaces gained a variadic option). Test coverage is strong: unit tests verify the header/query shape for every transport, exact request counts, token preservation, immutability/nil-receiver semantics, and the octet-stream→multipart fallback; the integration test exercises real ownership and POSIX boundaries.

I did not find functional bugs in the production change. Every public Files method threads withUser(f.user) — including transitively via WriteFiles → Write and Exists → Stat. ForUser correctly copies the struct (new pointer, shared connection), ForUser("") restores unscoped behavior, nil receivers are handled, and req.SetBasicAuth(user, "") produces the same header as the existing basicAuthUser(user) for non-empty users.

Findings

1. (Moderate) ForUser docs overstate the isolation guarantee — inline comment on sdk/go/files.go:58.

The godoc claims the view "executes every operation as user," and the README says "Execute all filesystem operations as a specific sandbox user." But as the PR's own integration test (TestIntegrationFilesForUserIsolation) documents, Files.Read/Files.Stat are privileged envd management operations: the selected user controls path expansion and ownership, but does not switch the envd process UID, so these endpoints do not enforce POSIX permissions. ForUser("nobody").Read(rootOnlyPath) is not denied the way running a command as nobody would be. Recommend adding a caveat to both the godoc and README that the user scope affects ownership/path context and that a real permission boundary requires Commands.

2. (Nit) The two transports carry identity differently. HTTP /files uses the username query param; the RPC endpoints use Basic auth; unscoped requests omit both. This matches the E2B convention and is intentional (and well-commented in envd.go), but it's a non-obvious split — worth a one-line note on ForUser so future maintainers keep both paths in sync if the convention changes.

3. (Nit) Integration-test template dependency. TestIntegrationFilesForUserIsolation hard-requires the template to provide a nobody user and fail fast otherwise. Acceptable for an opt-in integration test, but it means the new test is template-specific; a brief note in the test comment documenting that requirement would help other template authors.

Verification notes

• Request-count arithmetic in TestFilesForUserPropagatesIdentityToEveryTransport (11 = GET /files + 3× POST /files + ListDir + 2× Stat + Remove + Move + MakeDir + WatchDir) is consistent with the server handlers.
• Interface satisfaction holds: *Sandbox implements all three changed interfaces in the head tree; no other implementors exist in the repo.
• Commands().Run returns (result, nil) for non-zero exits, so the integration test's nobody cat expectation (transport error vs. exit code) is correct.

This review was generated by an AI assistant.

[cubesandboxbot]

Two nits on the new envd Basic-auth construction:

1. This builds the same header as the existing basicAuthUser helper (Basic base64(user + ":")) but bypasses it, so the envd Basic-header format now lives in two places. Consider req.Header.Set("Authorization", basicAuthUser(opts.user)) to keep a single source of truth — including the defaultEnvdUser fallback semantics.
2. Because this is guarded by opts.user != "", an empty user sends no Authorization header, whereas basicAuthUser("") would send Basic cm9vdDo= (root). So on the RPC path ForUser("root") is wire-different from unscoped. The comment documents the intent, but the README's ForUser("root") example (used to read /root/hello.txt) could lead users to think it's a no-op relative to the default — worth surfacing to SDK users.

[cubesandboxbot]

Consider documenting how the user is actually propagated, since it differs by transport: HTTP file operations (Read/Write/WriteFiles) send ?username=<user> on /files, while the filesystem RPC operations (List/Stat/Remove/Rename/MakeDir/WatchDir) send Authorization: Basic <user>:. The view is only consistent if envd honors both mechanisms. Also worth noting: ForUser("root") is wire-different from the unscoped default on the RPC path (explicit Basic root vs no header), even though both are semantically "root".

[wangxuw]

hi, PTAL when you have a moment
@wbzdssm @liciazhu

[luzhixing12345]

LGTM, this solution is quite elegant. I ran some tests, and they all passed.

I notice that in #1346 Acceptance Criteria part that you mention

An integration test demonstrates at least one filesystem operation against an envd deployment that otherwise returns no user specified.

@wangxuw Could you please add an integration test in integration_test.go to verify that Files.ForUser works against a real envd instance, including successful user-scoped file operations and correct permission isolation between different users. The current unit tests only validate the generated query parameters and headers.

[wangxuw]

LGTM, this solution is quite elegant. I ran some tests, and they all passed.

I notice that in #1346 Acceptance Criteria part that you mention

An integration test demonstrates at least one filesystem operation against an envd deployment that otherwise returns no user specified.

@wangxuw Could you please add an integration test in integration_test.go to verify that Files.ForUser works against a real envd instance, including successful user-scoped file operations and correct permission isolation between different users. The current unit tests only validate the generated query parameters and headers.

sure, added an integration test in the latest commit. The integration test passed via a TencentCloud AGS sandbox setup. @luzhixing12345

[cubesandboxbot]

Documentation accuracy: "executes every operation as user" overstates the isolation this view provides. Per the integration test added in this PR (TestIntegrationFilesForUserIsolation), Files.Read/Files.Stat are "privileged envd management operations" — the selected user controls path expansion and ownership, but these endpoints do not switch the envd process UID, so they are not a POSIX permission boundary. A caller doing sb.Files().ForUser("nobody").Read(rootOnlyPath) should not expect a permission denial (the test verifies the real boundary via Commands/cat, deliberately not via Files.Read).

Suggest a caveat such as: "runs operations in the filesystem context of user (ownership and path expansion). Note the envd file HTTP/RPC endpoints are privileged and do not enforce POSIX permissions — use Commands for a permission boundary." The same applies to the README's "Execute all filesystem operations as a specific sandbox user" line.