Skip to content

Instantly share code, notes, and snippets.

@darwin

darwin/improve1-plan.md

Created February 24, 2026 19:23
Show Gist options

  • Select an option

    Learn more about clone URLs
  • Save darwin/c76435ae72fc7206a3acd55e9ab5a603 to your computer and use it in GitHub Desktop.

Select an option

Learn more about clone URLs
Save darwin/c76435ae72fc7206a3acd55e9ab5a603 to your computer and use it in GitHub Desktop.
Download ZIP
Supex Improve 1 — Security hardening and protocol reliability
Raw
improve1-plan.md

Supex Improve 1 — Security hardening and protocol reliability

Format: Plan for plan-runner skill execution. Each ## Phase <code-name> section is one plan-runner invocation. Phases with ### Verify sections ending in test commands are automatically verifiable.

Project Overview

Cross-stack hardening pass across supex: path traversal defenses, connection reliability, protocol contract alignment, and defensive geometry validation. Covers sidecar (Rust), runtime (Ruby), driver (Python), and viewer (TypeScript).

Scope

  • In: path containment in sidecar recovery, runtime PathPolicy, batch screenshots, REPL, and driver VCAD tools; socket concurrency and singleton rotation; handshake and viewer relay contract alignment; sidecar lifecycle readiness; bridge legacy compat; mesh array guards
  • Out: new features, protocol redesign, OS-level sandboxing, multi-socket connection pools

Architecture

All changes operate within the existing supex architecture:

            AI Agent (Claude Code, MCP client)
                         |
                  [MCP Protocol (stdio)]
                         |
                supex Python MCP Driver          ← Phases socket-safety, driver-boundary, protocol-alignment
                /                    \
  [TCP JSON-RPC :9876]        [TCP JSON-RPC :9877]
         |                            |
  SketchUp Ruby Runtime       VCAD Rust Sidecar
  (bridge_server.rb)          (evaluator + mesh)   ← Phases sidecar-safety, protocol-alignment
         |                            |
  SketchUp Application         .cmp.oo files

Additional components:

  • Runtime PathPolicy → Phases path-policy, runtime-hardening
  • VCAD Viewer relay → Phase viewer-contract

Repositories

All changes in the supex repo: /Users/darwin/x/p/supex-ws/supex (branch dev)

Key Decisions (resolved open questions)

Question Decision Rationale
Quarantine vs delete invalid markers (01) Delete + log warning Quarantine adds complexity; logging is sufficient
Batch screenshot error surface (02) Structured error from batch_screenshot directly Consistent with existing tool error patterns
Separate validate_write_target! (03) Keep unified validate! Simpler API; write-target is an internal detail
Serialization vs future-map for socket concurrency (04) Strict serialization (lock) Expected traffic is low (human-driven MCP); simpler
Backward compat for old handshake format (05) No Internal protocol; sidecar and driver deploy together
Stderr handling strategy (06) Inherited stderr Simpler; Radar handles log aggregation
Legacy path deprecation warning (07) Yes, one warning per session Signal intent to remove without breaking flow
Additional workspace roots via env (08) No Single root sufficient; extra roots add complexity
Invalid REPL PID behavior (09) Fallback to Process.pid + warning Maintains compatibility while defending traversal
Mesh transport contract (10) dae_path file-based is canonical Simpler, avoids large inline payloads
Connection identity includes workspace (11) Yes Prevents stale connections after workspace switch
Malformed mesh error code (12) Dedicated MESH_MALFORMED code Machine-readable diagnostics

Phase Ordering

  1. Phase sidecar-safety — sidecar path containment + mesh guards (Rust, independent)
  2. Phase path-policy — PathPolicy symlink hardening + batch screenshot enforcement (Ruby runtime)
  3. Phase runtime-hardening — bridge legacy compat + REPL sanitization (Ruby runtime)
  4. Phase socket-safety — socket concurrency + connection singleton rotation (Python driver)
  5. Phase driver-boundary — VCAD source file workspace boundary (Python driver)
  6. Phase protocol-alignment — handshake contract + sidecar lifecycle readiness (Python driver + Rust sidecar)
  7. Phase viewer-contract — viewer relay contract + screenshot capability (Python driver + TypeScript viewer)

Phases 1–3 and 4–5 can run in parallel (different codebases). Phase 6 touches both driver and sidecar. Phase 7 depends on no other phase.

Phase sidecar-safety: Sidecar path containment and mesh array guards

Objective

Harden sidecar crash recovery so pending marker cleanup cannot delete files outside the temp root. Add defensive length validation for mesh arrays to prevent panics on malformed geometry buffers. Combines plans 01 and 12.

Specification

Recovery path containment (plan 01)

In evaluator.rs, the recover_incomplete_artifact_pairs function processes .pending marker files that reference mesh and manifest paths. Currently, a crafted marker could reference paths outside temp_dir.

Changes:

  • Add a shared helper is_inside_root(candidate: &Path, root: &Path) -> bool that canonicalizes both paths and checks containment.
  • Before any fs::remove_file in recovery, validate the target path is inside canonical temp_dir.
  • On invalid paths: skip file delete, emit warn!() log, still remove the marker file itself.
  • Invalid markers are deleted (not quarantined) — logging provides sufficient diagnostics.

Mesh array guards (plan 12)

In evaluator.rs and dae_export.rs, mesh processing assumes positions.len() % 3 == 0 and valid index ranges.

Changes:

  • Replace step_by(3) direct indexing with chunks_exact(3) where appropriate.
  • Add explicit guard: if positions.len() % 3 != 0 → return Err with MESH_MALFORMED error code.
  • Same for index buffer triangle-size assumptions.
  • Ensure both DAE export and bbox computation share compatible validation (validate once, early).
  • Error strategy: return structured internal error, never silently drop geometry.

Verify

cd /Users/darwin/x/p/supex-ws/supex && ./scripts/launch-tests.sh sidecar

Phase path-policy: PathPolicy symlink hardening and batch screenshot enforcement

Objective

Upgrade runtime PathPolicy to resolve symlinked parent directories before containment checks. Then enforce PathPolicy for batch screenshot output directories. Combines plans 03 and 02.

Specification

Symlink hardening (plan 03)

In path_policy.rb, the current resolve_path uses File.expand_path which does not resolve symlinks in parent directories. A symlinked parent can escape allowed roots.

Changes:

  • For write targets where the file may not yet exist: resolve and canonicalize the parent directory (File.realpath on nearest existing ancestor), then verify containment.
  • For broken symlinks and missing parent directories: deny with clear error message.
  • Keep unified validate! method (no separate validate_write_target!).
  • All existing call sites already pass workspace consistently — verify this holds.

Batch screenshot path enforcement (plan 02)

In batch_screenshot.rb (or tools.rb), the take_batch_screenshots tool accepts output_dir which is not validated against PathPolicy.

Changes:

  • Add PathPolicy.validate! call for caller-provided output_dir before FileUtils.mkdir_p.
  • Ensure per-shot paths derived from output dir stay within validated root.
  • Default destination via PathPolicy.default_tmp_dir(workspace) continues to work.
  • On rejection: return structured PATH_NOT_ALLOWED error directly from batch_screenshot.
  • Centralize output dir resolution to avoid duplicate logic.

Verify

cd /Users/darwin/x/p/supex-ws/supex && ./scripts/launch-tests.sh runtime

Phase runtime-hardening: Bridge legacy compat and REPL session sanitization

Objective

Fix the broken legacy command request path in the runtime bridge. Harden REPL session directory creation against path traversal. Combines plans 07 and 09.

Specification

Bridge legacy command compat (plan 07)

In bridge_server.rb, the handle_legacy_command code path is broken because it doesn't receive context (including workspace).

Changes:

  • Update handle_legacy_command to accept and forward context from the connection state.
  • Ensure converted tool request preserves request id and arguments correctly.
  • Handle nil/empty legacy parameters safely in conversion.
  • Emit one deprecation warning per session when legacy path is used.
  • Do not leak sensitive arguments in deprecation log.

REPL session path sanitization (plan 09)

In repl_server.rb, session directory paths are constructed from client-provided pid values during hello. A crafted PID like ../../etc could escape SNIPPETS_DIR.

Changes:

  • Add strict PID validation: numeric characters only, bounded length (max 10 digits).
  • On invalid PID: fallback to Process.pid with warning (maintains compatibility).
  • Build session directory name from sanitized tokens only.
  • Canonicalize the created session path and verify starts_with?(canonical_snippets_root).
  • Reject any session path that escapes snippets root with structured JSON-RPC error.

Verify

cd /Users/darwin/x/p/supex-ws/supex && ./scripts/launch-tests.sh runtime

Phase socket-safety: Socket RPC concurrency and connection singleton rotation

Objective

Make SketchUp and VCAD socket clients concurrency-safe with serialized send+receive and strict response ID matching. Fix singleton connection factories to respect host/port changes. Combines plans 04 and 11.

Specification

Socket concurrency and ID matching (plan 04)

In sketchup_connection.py and vcad_connection.py, concurrent tool calls can cause one request to consume another's response. Response IDs are not validated.

Changes:

  • Add a threading Lock around each connection's send+receive cycle (strict serialization).
  • After receiving a response, verify response["id"] == request_id; raise ProtocolError on mismatch.
  • Retry logic must not replay on corrupted socket state — close and reconnect instead.
  • Add defensive logging for ID mismatches and lock contention events.

Connection singleton rotation (plan 11)

Singleton connection factories cache by agent name only. If host or port changes (e.g., SketchUp restarted on different port), the stale connection is reused.

Changes:

  • Extend singleton cache key to include (host, port, workspace) in addition to existing identity.
  • On endpoint mismatch: disconnect old connection, create new one.
  • Log when endpoint rotation occurs.
  • For VCAD connections: include workspace in identity since different workspaces may use different sidecar instances.

Verify

cd /Users/darwin/x/p/supex-ws/supex && ./scripts/launch-tests.sh driver

Phase driver-boundary: Driver workspace boundary for VCAD source files

Objective

Add explicit workspace boundary validation for VCAD source file reads in driver MCP tools. Prevents MCP callers from reading files outside the configured workspace.

Specification

In vcad_tools.py, tools like vcad_place and vcad_update read source_file from disk before sending to sidecar. No boundary check exists.

Changes:

  • Add a reusable validate_workspace_path(path: str, workspace: Path) -> Path helper in the driver MCP layer.
  • Canonicalize the path (Path.resolve()) and verify it starts with canonical workspace root.
  • Enforce validation before every open(source_file) in vcad tool flows.
  • Normalize relative paths against workspace; deny traversal and absolute paths outside root.
  • If SUPEX_WORKSPACE is not set: deny all source file reads (fail closed).
  • Route denials through existing error builder with PATH_NOT_ALLOWED code and operation context.
  • No additional roots via env config — single workspace root is the boundary.

Verify

cd /Users/darwin/x/p/supex-ws/supex && ./scripts/launch-tests.sh driver

Phase protocol-alignment: Handshake contract and sidecar lifecycle readiness

Objective

Unify VCAD handshake protocol_version format across driver, sidecar, and JSON schemas. Upgrade sidecar lifecycle so process liveness means functional readiness. Combines plans 05 and 06.

Specification

Handshake contract alignment (plan 05)

Driver, sidecar, and schema currently disagree on protocol_version encoding.

Changes:

  • Canonical format: "major.minor" string (e.g. "1.0"). Lock in docs and code.
  • Sidecar hello parser: require string format, reject integer or malformed values with explicit error.
  • Driver hello payload: emit string format; parser expects string back.
  • Align sidecar hello response payload with docs/contracts/v1/handshake.schema.json.
  • Major-version mismatch → fail fast on both sides.
  • No backward compatibility for old format — internal protocol, deployed together.

Sidecar lifecycle readiness (plan 06)

In vcad_sidecar.py, ensure_running only checks poll() == None (process alive), not whether the sidecar can actually handle requests. Stdout/stderr pipes can block under heavy logging.

Changes:

  • After spawn: perform readiness probe (TCP connect + hello/ping with bounded retries, e.g. 5 retries, 500ms apart).
  • If process alive but unresponsive after probe timeout: kill and restart.
  • Replace PIPE stderr strategy with inherited stderr (process writes to parent's stderr).
  • Improve startup error reporting: explicit messages for "process died during startup" vs "started but unresponsive".
  • Clean stop must still avoid zombie process state.

Verify

cd /Users/darwin/x/p/supex-ws/supex && ./scripts/launch-tests.sh driver
cd /Users/darwin/x/p/supex-ws/supex && ./scripts/launch-tests.sh sidecar

Phase viewer-contract: Viewer relay contract and screenshot capability

Objective

Align viewer relay schemas with actual runtime payloads. Fix screenshot capability — either implement it or stop advertising it. Combines plan 10.

Specification

In the viewer relay, schemas/examples may not match actual mesh.update and scene.snapshot payloads. Screenshot capability is advertised but may not be implemented.

Changes:

  • Canonical mesh transport contract: dae_path file-based. Update docs/contracts/v1/viewer-relay.schema.json and examples to match.
  • Ensure driver relay emitter (vcad_viewer_relay.py) and viewer consumer (DriverRelayClient.ts) conform to schema.
  • For screenshot: if the capture path is not implemented, remove screenshot from announced capabilities in viewer hello. Re-add when actually implemented.
  • If screenshot IS implemented: ensure vcad_viewer_screenshot MCP endpoint returns non-empty image data when capability is enabled.
  • Add contract validation tests for relay messages (schema-compatible payload exchange).
  • Prevent schema drift with automated contract tests.

Verify

cd /Users/darwin/x/p/supex-ws/supex && ./scripts/launch-tests.sh driver
cd /Users/darwin/x/p/supex-ws/supex && ./scripts/launch-tests.sh viewer
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment