docs: add OpenCode integration example#766
Conversation
Assisted-by: Codex:GPT-5 Signed-off-by: Sam-Hui-dot <19303092837@163.com>
PR Review:
|
|
|
||
| try: | ||
| return sandbox.commands.run(command, **kwargs) | ||
| except TypeError as exc: |
There was a problem hiding this comment.
The TypeError catch uses a string-match heuristic ("envs" not in str(exc)) against the exception message. If sandbox.commands.run raises a TypeError for an unrelated reason where the word "envs" happens to appear in the error text, this silently takes the fallback path and likely raises a confusing follow-up error. Consider probing the SDK API once at module import time instead of per-call exception-driven fallback.
| return getattr(sandbox, "sandbox_id", getattr(sandbox, "id", "unknown")) | ||
|
|
||
|
|
||
| def secret_is_present(value: str | None) -> bool: |
There was a problem hiding this comment.
secret_is_present() (line 64) and redact_env() (line 68) are defined here but never imported or called by any script in this package. Remove if unused, or add a comment explaining intended future use.
| return value | ||
|
|
||
|
|
||
| def optional(name: str, default: str = "") -> str: |
There was a problem hiding this comment.
Using or here means env vars explicitly set to "", "0", or "false" are indistinguishable from "not set" and silently replaced with the default. For OPENCODE_DISABLE_AUTO_UPDATE, a user who unsets it would unintentionally revert to "1" instead of empty. Consider os.environ.get(name) or explicit None-check if distinguishing empty-from-unset matters.
| return explicit | ||
| provider_specific = os.environ.get(f"{provider_name.upper()}_BASE_URL") | ||
| return provider_specific or "" | ||
|
|
There was a problem hiding this comment.
The {PROVIDER_NAME}_BASE_URL fallback (e.g., OPENAI_BASE_URL) is undocumented — it's not mentioned in .env.example or any of the four doc files. Users working with custom endpoints may not discover it. Consider adding it as a commented option in .env.example.
| python3 \ | ||
| python3-pip \ | ||
| ripgrep \ | ||
| && curl -fsSL "https://deb.nodesource.com/setup_${NODE_MAJOR}.x" | bash - \ |
There was a problem hiding this comment.
Piping a fetched script directly to bash offers no integrity verification (no checksum, no GPG signature). If the NodeSource CDN were compromised, the resulting template image could contain injected code. For an example this is tolerable, but consider documenting the risk so downstream users know not to cargo-cult this into production pipelines.
| @@ -0,0 +1,60 @@ | |||
| # Copyright (c) 2026 Tencent Inc. | |||
There was a problem hiding this comment.
import env_utils works only when run from examples/opencode-integration/. Other example test suites in this repo (e.g., test_dev_sidecar.py) explicitly insert the directory into sys.path for portability. Without this, the tests cannot be discovered by a parent-directory pytest invocation or CI runner.
|
|
||
| try: | ||
| return sandbox.commands.run(command, **kwargs) | ||
| except TypeError as exc: |
There was a problem hiding this comment.
The TypeError catch uses a string-match heuristic ("envs" not in str(exc)) against the exception message. If sandbox.commands.run raises a TypeError for an unrelated reason where the word "envs" happens to appear in the error text, this silently takes the fallback path and likely raises a confusing follow-up error. Consider probing the SDK API once at module import time instead of per-call exception-driven fallback.
| return getattr(sandbox, "sandbox_id", getattr(sandbox, "id", "unknown")) | ||
|
|
||
|
|
||
| def secret_is_present(value: str | None) -> bool: |
There was a problem hiding this comment.
secret_is_present() (line 64) and redact_env() (line 68) are defined here but never imported or called by any script in this package. Remove if unused, or add a comment explaining intended future use.
| return value | ||
|
|
||
|
|
||
| def optional(name: str, default: str = "") -> str: |
There was a problem hiding this comment.
Using or here means env vars explicitly set to "", "0", or "false" are indistinguishable from "not set" and silently replaced with the default. For OPENCODE_DISABLE_AUTO_UPDATE, a user who unsets it would unintentionally revert to "1" instead of empty. Consider os.environ.get(name) or explicit None-check if distinguishing empty-from-unset matters.
| return explicit | ||
| provider_specific = os.environ.get(f"{provider_name.upper()}_BASE_URL") | ||
| return provider_specific or "" | ||
|
|
There was a problem hiding this comment.
The {PROVIDER_NAME}_BASE_URL fallback (e.g., OPENAI_BASE_URL) is undocumented — it's not mentioned in .env.example or any of the four doc files. Users working with custom endpoints may not discover it. Consider adding it as a commented option in .env.example.
| python3 \ | ||
| python3-pip \ | ||
| ripgrep \ | ||
| && curl -fsSL "https://deb.nodesource.com/setup_${NODE_MAJOR}.x" | bash - \ |
There was a problem hiding this comment.
Piping a fetched script directly to bash offers no integrity verification (no checksum, no GPG signature). If the NodeSource CDN were compromised, the resulting template image could contain injected code. For an example this is tolerable, but consider documenting the risk so downstream users know not to cargo-cult this into production pipelines.
| @@ -0,0 +1,60 @@ | |||
| # Copyright (c) 2026 Tencent Inc. | |||
There was a problem hiding this comment.
import env_utils works only when run from examples/opencode-integration/. Other example test suites in this repo (e.g., test_dev_sidecar.py) explicitly insert the directory into sys.path for portability. Without this, the tests cannot be discovered by a parent-directory pytest invocation or CI runner.
Assisted-by: Codex:GPT-5 Signed-off-by: Sam-Hui-dot <19303092837@163.com>
|
|
||
|
|
||
| def stream_writer(stream) -> Callable[[object], None]: | ||
| def write(chunk: object) -> None: |
There was a problem hiding this comment.
Security: streaming output does not redact secrets
stream_writer writes directly to sys.stdout/sys.stderr without passing through redact_secrets(). If the agent executes a command that echoes the API key (e.g., echo $OPENAI_API_KEY), the key is printed in plaintext on the terminal.
Unlike ensure_success (which redacts output before raising), the streaming path is unguarded. Consider wrapping the writer to also redact secrets, or add a documentation note that run_opencode.py / resume_opencode.py are for local trusted use only and shared clusters should use network_policy.py (which is immune to this since it uses CubeEgress injection).
| return bool(value and value.strip() and not value.strip().startswith("<")) | ||
|
|
||
|
|
||
| def redact_secrets(text: str) -> str: |
There was a problem hiding this comment.
redact_secrets iterates in arbitrary order — can leave partial secrets unreplaced
(_opencode_common.py:101-106)
The function calls str.replace(value, "<redacted>") for each env var in insertion order. If one secret value is a prefix of another (e.g., A_KEY=sk-foo and B_KEY=sk-foobar), the shorter value's replacement corrupts the longer one in the string, leaving <redacted>bar in the output.
This was flagged independently by both the security and code-quality reviewers. The fix is to sort values by length descending before processing (longest match first):
| return {key: value for key, value in kwargs.items() if key in params} | ||
|
|
||
|
|
||
| def _is_secret_env(name: str) -> bool: |
There was a problem hiding this comment.
Overly broad substring matching
any(marker in name.upper() ...) matches on substring containment, not suffix/prefix. An innocuous env var like CSRF_TOKEN_EXPIRY_SECONDS would be treated as a secret because it contains both TOKEN and SECRET, causing 3600 to be redacted from output. Consider using suffix matching (endswith) instead.
| ) | ||
|
|
||
|
|
||
| def load_local_dotenv() -> None: |
There was a problem hiding this comment.
Silent skip when no .env is found
If neither candidate .env exists (e.g., user forgot to create it), the function returns silently. The next call to required() then raises SystemExit("Missing required environment variable: ...") with no indication that the root cause is a missing .env file. Consider printing a warning to stderr when none of the candidate paths exist.
| sandbox_id = sandbox_identifier(sandbox) | ||
| print(f"Sandbox ready: {sandbox_id}") | ||
|
|
||
| version_result = run_command(sandbox, "opencode --version", timeout=60) |
There was a problem hiding this comment.
Unnecessary version-check round trip on every run
opencode --version is already pinned by OPENCODE_VERSION in the Dockerfile and verified at build time. This adds 0.1–2s of overhead per invocation with no diagnostic value that the build-time check doesn't already provide. Consider gating behind a --verbose flag or removing it.
Assisted-by: Codex:GPT-5 Signed-off-by: Sam-Hui-dot <19303092837@163.com>
| return bool(value and value.strip() and not value.strip().startswith("<")) | ||
|
|
||
|
|
||
| def redact_secrets(text: str) -> str: |
There was a problem hiding this comment.
Performance: redact_secrets() re-scans all env vars on every streaming chunk
This is called from stream_writer() on every stdout/stderr chunk during a long OpenCode run. It iterates os.environ, filters by secret-suffix heuristics, sorts by length, and performs O(n*m) string replacement — every single chunk. The secret values are static for the process lifetime.
Suggestion: Cache the discovered values with lru_cache or compute them once at module level:
@lru_cache(maxsize=1)
def _get_secret_values() -> tuple[str, ...]:
values = {
value for name, value in os.environ.items()
if _is_secret_env(name) and _secret_is_present(value)
}
return tuple(sorted(values, key=len, reverse=True))Then redact_secrets only iterates the cached tuple. This eliminates thousands of redundant env-var scans during a long streaming session.
| kwargs = {"cwd": cwd, "timeout": timeout, "user": user} | ||
| kwargs = {key: value for key, value in kwargs.items() if value is not None} | ||
| if envs: | ||
| kwargs[_command_env_kwarg(type(sandbox.commands))] = envs |
There was a problem hiding this comment.
Debugging concern: stream=True may clear stdout/stderr from the result object
When stream=True, the on_stdout/on_stderr callbacks consume the output. The returned result object may have empty .stdout and .stderr attributes (depending on the e2b/cubesandbox SDK behavior). If the command subsequently fails, ensure_success() reports empty stdout/stderr, making debugging very difficult.
Suggestion: Buffer the stream output in the stream_writer closure and attach it back to the result, or log the last N lines to a separate buffer so failure diagnostics are useful.
| if provider_name == "anthropic": | ||
| return [ | ||
| {"header": "x-api-key", "secret": secret, "format": "${SECRET}"}, | ||
| { |
There was a problem hiding this comment.
Hardcoded Anthropic API version — will go stale over time
"2023-06-01" is hardcoded as the Anthropic API version. When Anthropic deprecates this version in the future, the script will silently send an outdated version header and receive errors, with no obvious connection to this value. Additionally, registering it as a CubeEgress "secret" (via the "secret" key in the inject dict) is semantically misleading — it's a version identifier, not a credential.
Suggestion: Pull from an environment variable (e.g., ANTHROPIC_VERSION) with "2023-06-01" as a default, so it can be updated without a code change.
| return " ".join(shlex.quote(arg) for arg in args) | ||
|
|
||
|
|
||
| def shell_join(*parts: str) -> str: |
There was a problem hiding this comment.
shell_join() trusts callers to sanitize inputs
All current callers properly use shlex.quote() on values before joining, so there's no exploitable path today. However, shell_join is a public exported function that performs no escaping itself. If a future caller forgets to quote, the result is shell injection.
Suggestion: Either (a) document the expectation by renaming to _shell_join_unsafe with a docstring noting that inputs must be pre-quoted, or (b) accept a list of arguments rather than pre-quoted strings and quote internally.
| gnupg \ | ||
| jq \ | ||
| less \ | ||
| procps \ |
There was a problem hiding this comment.
Dockerfile layer caching: apt packages and NodeSource setup in one layer
If NODE_MAJOR or OPENCODE_VERSION changes, the merged apt-get install ... curl nodesource.com && apt-get install nodejs layer is fully invalidated, causing all apt packages to reinstall even though none changed.
Suggestion: Split into two layers:
# Layer 1: base packages (stable)
RUN apt-get update && apt-get install -y --no-install-recommends \
bash ca-certificates curl git gnupg jq less procps \
python3 python3-pip ripgrep \
&& rm -rf /var/lib/apt/lists/*
# Layer 2: NodeSource (changes with NODE_MAJOR)
RUN curl -fsSL "https://deb.nodesource.com/setup_${NODE_MAJOR}.x" | bash - \
&& apt-get install -y --no-install-recommends nodejs \
&& rm -rf /var/lib/apt/lists/*This preserves the cached base-package layer across Node version bumps — minor for a demo but helpful during iterative CI builds.
|
|
||
|
|
||
| @lru_cache(maxsize=None) | ||
| def _command_env_kwarg(commands_type: type) -> str: |
There was a problem hiding this comment.
Fragile SDK introspection for env parameter naming
_command_env_kwarg() reflects on the e2b/cubesandbox SDK's commands.run signature to determine whether the parameter is called env or envs. This couples the example to internal SDK naming. If a future SDK version renames the parameter (e.g., to environment), _filter_supported_kwargs silently drops the env dict and the agent runs without credentials.
Suggestion: Add a comment documenting which SDK versions use which parameter name, so upgrade diagnostics are clearer. A more robust approach would be to try both names and accept whichever works.
|
Refs #644 |
|
Hello, thank you for your contribution! Since we have a large number of participants in this event, to speed up the PR review process, could you please attach some screenshots and logs (both are needed) showing the verification process and results for this PR? Thank you for your participation! |
Refs #644
Summary
This PR adds an OpenCode integration example for CubeSandbox under
examples/opencode-integration.The example covers template construction, headless OpenCode execution, pause/resume persistence, restricted egress, CubeEgress credential injection, and bilingual documentation.
What It Demonstrates
cubesandbox-baseopencode run --auto --format json/workspacecalculator.addpython3 -m unittest discover -vresult.mdcontainsOPENCODE_CUBE_OKOPENCODE_RESUME_OKallow_internet_access=FalseOPENCODE_EGRESS_OKdocs/guide/integrationsValidation
Static checks:
cd examples/opencode-integration python3 -m py_compile env_utils.py _opencode_common.py run_opencode.py resume_opencode.py network_policy.py python3 -m unittest test_env_utils test_commands -v bash -n ./build-template.sh git diff --check -- examples/opencode-integration docs/guide/integrations docs/zh/guide/integrationsImage checks:
docker build --platform linux/amd64 -t opencode-cube:verify . docker run --rm opencode-cube:verify opencode --version docker run --rm opencode-cube:verify node --version docker run --rm opencode-cube:verify python3 --versionEnd-to-end local CubeSandbox checks:
Observed result:
dev-envnpm run docs:buildNotes
Refs #644is used instead of a closing keyword because 【2026犀牛鸟issue】CubeSandbox 与 Claude Code / CodeBuddy / OpenCode 集成 #644 is a multi-part participation issue.Assisted-by: Codex:GPT-5