Skip to content

fix(codemode): harden saved snippet execution - #682

Merged
jmagar merged 22 commits into
mainfrom
fix/snippet-runtime-hardening-20260916
Sep 19, 2026
Merged

jmagar merged 22 commits into
mainfrom
fix/snippet-runtime-hardening-20260916

Conversation

@jmagar

@jmagar jmagar commented Sep 16, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • keep saved snippet source on the execution plane instead of model-facing catalog/results
  • replace the legacy 20 KiB snippet ceiling with Code Mode's 1 MiB source contract and align composed snippet budgets/defaults
  • parse saved JavaScript during validation without executing it
  • enforce exact saved-snippet tool declarations for native execution and one-shot cached catalog construction
  • preserve compact results under high-fanout trace pressure by shedding optional trace params first
  • remove repeated/recovered startup warning spam while retaining actionable failure telemetry
  • add composable homelab SSH/Docker inventory snippets with exact claude-macpoo dependencies

Verification

  • labby-codemode snippet suite: 30 passing
  • scoped cached catalog oracle: passing
  • gateway spawn guard, lifecycle, stdio and reprobe suites: passing
  • native saved-snippet scope oracle: passing
  • feature-gated Q3 dependent-call oracle: passing
  • isolated cold branch CLI smoke: exit 0, 0 stderr lines, no truncation marker, 138 real containers inventoried
  • live MCP smoke: 138 containers across Tootie/Squirts/Steamyy/Tower, 0 inventory errors, full artifact written, <=20 log lines/container

Notes

The active root-owned Labby system service is intentionally not replaced from the unprivileged labby account. Final saved snippets are installed and validated live; binary rollout should use the normal privileged deployment path after merge.

@github-actions github-actions Bot added the documentation Improvements or additions to documentation label Sep 16, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Unresolved critical and moderate review findings remain in budget enforcement, tool scoping, telemetry, and homelab inventory snippets.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

This pull request hardens saved Code Mode snippet execution and adds reusable homelab SSH/Docker inventory snippets.

Changes:

  • Raises source and composition limits with parse-only validation.
  • Adds exact tool scoping, response-budget handling, and lifecycle telemetry improvements.
  • Documents and implements homelab SSH discovery and Docker inventory workflows.
File summaries
File Reviewed changes and findings
plugins/labby/skills/using-labby/references/config-reference.md Updates source-limit configuration documentation.
plugins/labby/skills/using-labby/references/code-mode.md Updates Code Mode limits and execution guidance.
docs/snippets/README.md Documents saved-snippet execution and tool declarations.
docs/snippets/homelab-ssh-targets.md Adds SSH discovery. Critical: terminate SSH options before aliases. Moderate: preserve missing-command fields and report traversal truncation.
docs/snippets/homelab-docker-inventory.md Adds multi-host Docker inventory. Critical: redact full ssh_config from artifacts.
docs/snippets/docker-host-inventory.md Adds per-host Docker inventory. Critical: disable SSH agent forwarding. Moderate: preserve optional-command field positions.
docs/services/SNIPPETS.md Updates snippet service documentation.
docs/dev/CODE_MODE.md Updates Code Mode development guidance.
crates/labby/src/dispatch/snippets/dispatch.rs Executes saved snippets with scoped dependencies. Moderate: enforce the effective source limit before execution.
crates/labby/src/dispatch/setup/settings.rs Updates configuration exposure.
crates/labby/src/config.rs Updates Code Mode defaults.
crates/labby-runtime/src/gateway_config.rs Validates source-size configuration.
crates/labby-gateway/src/upstream/pool/stdio_transport.rs Adjusts child-termination telemetry. Moderate: retain warnings for unexpected failures.
crates/labby-gateway/src/upstream/pool/probe.rs Updates upstream probing behavior.
crates/labby-gateway/src/upstream/pool/lifecycle_compat.rs Updates lifecycle compatibility handling.
crates/labby-gateway/src/upstream/pool/connect.rs Updates connection behavior.
crates/labby-gateway/src/security/spawn_guard.rs Updates spawn-guard behavior.
crates/labby-gateway/src/gateway/manager/tests/code_mode.rs Adds Code Mode catalog coverage.
crates/labby-gateway/src/gateway/manager/code_mode_runtime.rs Filters cached catalogs and adjusts telemetry. Moderate: retain an actionable partial-probe failure signal.
crates/labby-gateway/src/gateway/code_mode/search.rs Builds scoped catalogs. Moderate: filter returned tools by the full declaration scope.
crates/labby-codemode/src/truncate.rs Preserves compact responses under trace pressure.
crates/labby-codemode/src/snippet/tool_declarations.rs Validates exact tool declarations.
crates/labby-codemode/src/snippet/store.rs Adds snippet limits and validation. Moderate: enforce configured lower source caps and avoid exposing filesystem paths in errors.
crates/labby-codemode/src/runner_drive/artifacts.rs Resolves nested snippets and enforces execution budgets.
crates/labby-codemode/src/config.rs Defines source and composition budgets. Critical: align the generated sandbox cap with the advertised 1 MiB budget.
config/config.example.toml Updates example configuration.
Review details

Suppressed comments (7)

crates/labby-codemode/src/snippet/store.rs:24

  • The saved-snippet path only forwards config.max_source_bytes to the broker; unlike direct Code Mode requests, this broker path does not enforce that configurable lower limit (the runner enforces only the separate 1 MiB composed-snippet budget). A host configured with, for example, a 128 KiB source cap can therefore execute a nearly 1 MiB saved snippet, contradicting the comment here and bypassing the operator's resource limit. Enforce the configured cap on the resolved saved-snippet source before wrapping/execution and add a lower-limit test.
/// legacy cap. Hosts may still configure a lower `code_mode.max_source_bytes`,
/// which is enforced when the snippet executes.
const MAX_SNIPPET_CODE_BYTES: usize = crate::config::MAX_SOURCE_BYTES;

crates/labby-codemode/src/snippet/store.rs:353

  • This not-found error is serialized to callers of the snippets action and now includes the server's absolute user and built-in snippet directories. That exposes deployment filesystem layout to remote clients for a condition that only needs a stable missing-name error; keep the paths in server-side diagnostics rather than the model-facing message.
        message: format!(
            "snippet `{name}` not found; searched user snippets at `{}` and built-ins at `{}`",
            user_dir.display(),
            builtin_dir.display()
        ),

crates/labby-gateway/src/gateway/code_mode/search.rs:137

  • The scoped render path only restricts which upstreams are contacted; it never applies the declaration's exact scope.allows predicate to the returned tools. Thus a declaration such as omega::ping still injects every discovered tool in omega into the runner catalog: direct calls are denied later, but codemode.search and generated helpers can expose undeclared schemas and defeat the exact-declaration/minimal-catalog contract. Filter the returned tools by the full scope before rendering.
) -> Result<ToolsRender, ToolError> {
    let raw_tools = if use_cache {
        manager
            .code_mode_catalog_tools_cached_allowed(Some(owner), oauth_subject, allowed_upstreams)
            .await?
    } else {
        manager

crates/labby-gateway/src/gateway/manager/code_mode_runtime.rs:621

  • A failed cold probe is now logged only at DEBUG, and the method still returns a partial catalog when another upstream connects. The failed upstream is not included in the response or an aggregate warning, so its tools can disappear silently in production unless DEBUG logging is enabled; retain one actionable warning/partial-failure signal while avoiding repeated per-upstream startup spam.
                Ok(Some((upstream, _, Err(error)))) => {
                    outstanding.remove(&upstream.name);
                    tracing::debug!(
                        surface = "dispatch",
                        service = "gateway",

docs/snippets/docker-host-inventory.md:112

  • Shell quoting protects the Bash command but not OpenSSH's option parser: a caller-provided alias such as -oProxyCommand=... is still interpreted as an SSH option after quote removal, allowing an unintended local proxy command. Terminate SSH options with -- before the alias, and apply the same guard to every alias-based SSH invocation.
				command: "ssh " + opts + " " + q(i.alias) + " " + q(r),

docs/snippets/homelab-ssh-targets.md:253

  • Shell quoting protects the Bash command but not OpenSSH's option parser: a configured or extra host alias beginning with an option can still alter this SSH invocation after quote removal. Terminate SSH options with -- before target.alias so input cannot inject client options.
						"ssh " + opts + " " + quote(target.alias) + " " + quote(probe),

docs/snippets/homelab-ssh-targets.md:231

  • These SSH commands inherit ForwardAgent from the user's configuration. If a selected alias enables agent forwarding, the read-only discovery probe exposes the controller's agent to the remote host; explicitly disable agent forwarding for this workflow.
		"-o BatchMode=yes",
  • Files reviewed: 26/26 changed files
  • Comments generated: 9
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread crates/labby-codemode/src/config.rs
Comment thread docs/snippets/docker-host-inventory.md
Comment thread docs/snippets/homelab-docker-inventory.md Outdated
Comment thread docs/snippets/homelab-ssh-targets.md Outdated
Comment thread crates/labby-gateway/src/upstream/pool/stdio_transport.rs Outdated
Comment thread crates/labby/src/dispatch/snippets/dispatch.rs Outdated
Comment thread docs/snippets/docker-host-inventory.md Outdated
Comment thread docs/snippets/homelab-ssh-targets.md
Comment thread docs/snippets/homelab-ssh-targets.md Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Unresolved critical and moderate findings remain in tool scoping, truncation, source validation, diagnostics, and homelab snippets.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (8)

crates/labby-codemode/src/snippet/store.rs:24

  • This now accepts a full MAX_SOURCE_BYTES, but saved execution wraps every snippet with an input/async-function wrapper before CodeModeBroker enforces config.max_source_bytes. A snippet at the advertised 1 MiB boundary (or with sufficiently large input) passes storage validation and is then rejected before running. Reserve wrapper/input overhead or validate the final wrapped source so the storage and execution contracts agree.
const MAX_SNIPPET_CODE_BYTES: usize = crate::config::MAX_SOURCE_BYTES;

crates/labby-codemode/src/snippet/store.rs:237

  • read_resolved now compiles the extracted JavaScript during validate_snippet_body, and callers then invoke code_for_snippet, which compiles the same source again. For example, every gateway snippet resolve followed by execution pays for two Javy parses; with the new 1 MiB limit this is an avoidable per-run cost. Return/reuse the validated extracted source, or separate metadata validation from execution-source validation.
    let code = normalize_snippet_code(&code).to_string();

crates/labby-codemode/src/snippet/store.rs:586

  • A valid saved snippet ending in a // comment is rejected here: normalize_snippet_code trims the final newline, so appending ); on the same line lets the comment swallow the validator's closing delimiter. The execution normalizer already accepts this form, so put the generated closing delimiter on a new line (or strip trailing comments consistently).
    let source = format!("export default ({code});");
    runtime
        .compile_to_bytecode("snippet-validation.js", &source)

crates/labby-gateway/src/gateway/manager/code_mode_runtime.rs:624

  • This downgrade leaves the all-failed one-shot path without an actionable warning: code_mode_catalog_tools_cached_allowed returns earlier when nothing connected, before the summary warning below this block, so this DEBUG event is the only log containing the failed upstream name. Keep a warning summary on that path (or preserve WARN here) while avoiding repeated warnings for recovered/suppressed probes.
                    tracing::debug!(
                        surface = "dispatch",
                        service = "gateway",
                        action = "code_mode.catalog_cache",
                        upstream = %upstream.name,

crates/labby-gateway/src/security/spawn_guard.rs:114

  • WARNED_COMMANDS grows for every distinct bypassed command and is never evicted or bounded. Because bypass skips all validation, a long-lived process that accepts unique command strings can grow this process-wide BTreeSet indefinitely; use a bounded dedupe or aggregate warning instead.
    static WARNED_COMMANDS: OnceLock<Mutex<BTreeSet<String>>> = OnceLock::new();
    let warned = WARNED_COMMANDS.get_or_init(|| Mutex::new(BTreeSet::new()));
    let first_for_command = warned
        .lock()
        .unwrap_or_else(|poisoned| poisoned.into_inner())
        .insert(command.to_string());

crates/labby/src/dispatch/snippets/dispatch.rs:305

  • Using ToolScope::default() here drops the caller's route/upstream policy. MCP snippets.exec/snippets.test reach this helper through generic dispatch, which supplies TrustedLocal, so a protected route that exposes the snippets service can execute an undeclared snippet—or a declaration naming another upstream—against any configured upstream. Thread the route-derived caller scope through these actions and intersect the declaration with it; otherwise the documented "declaration can only narrow caller authority" invariant is not enforced.
        .map(|declared| declared.intersect(&ToolScope::default()))

docs/snippets/homelab-ssh-targets.md:204

  • When ssh_config is supplied, discovery reads and parses root, but this ssh -G invocation omits -F root; the subsequent probe invocation has the same omission. Aliases can therefore be discovered from a custom file but resolved with the controller's default HostName/User/IdentityFile settings. Pass the selected config path to both SSH commands.
						command: "ssh -G -- " + quote(target.alias),

docs/snippets/homelab-ssh-targets.md:133

  • Relative Include expansion is wrong when the root ssh_config is a bare filename: with no /, lastIndexOf is -1, so this slice length becomes 1 and resolves config + included.conf as cincluded.conf. Use an empty directory prefix when from has no slash so the documented non-glob include expansion works for relative config paths.
				: from.slice(0, Math.max(0, from.lastIndexOf("/")) + 1) + p;
  • Files reviewed: 32/32 changed files
  • Comments generated: 3
  • Review effort level: Lite

Comment thread crates/labby-codemode/src/truncate.rs
Comment thread crates/labby/src/dispatch/snippets/dispatch.rs Outdated
Comment thread docs/snippets/homelab-docker-inventory.md Outdated
@jmagar
jmagar requested a lite review from Copilot September 17, 2026 13:56

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@jmagar
jmagar enabled auto-merge (squash) September 19, 2026 02:34
@jmagar
jmagar merged commit 79f10df into main Sep 19, 2026
70 checks passed
@jmagar
jmagar deleted the fix/snippet-runtime-hardening-20260916 branch September 19, 2026 06:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants