Open review findings

Status: Open — five confirmed findings need fixes; two prior catalog rows verified resolved and dropped

Purpose

This page is the consolidated list of acknowledged review findings that still need verification or follow-up. It is a catalog, not a design doc. When a finding graduates into concrete work, move it to a dedicated roadmap page; when a finding is fixed, remove it rather than keeping a stale resolved entry.

Resolved since last pass

Source-verified against the current tree and dropped from the catalog:

• Launcher preview and mount-resolution parity. AppConfig::resolve_mount_rows in crates/jackin-config/src/app_config/mounts.rs is the single function both the workspace details-pane preview (role_scoped_mount_count in crates/jackin-console/src/tui/screens/workspaces/view.rs) and the real launch resolver (resolve_load_workspace in crates/jackin-config/src/resolve.rs) call. Selector-scoped global mounts cannot drift between preview and launch because there is only one resolution path; test preview_shows_unscoped_global_mounts_without_role_ambiguity_text confirms scoped-only mounts are excluded from the unscoped preview line.
• Workspace role/agent selectors. AppConfig::create_workspace and AppConfig::edit_workspace in crates/jackin-config/src/app_config/workspaces.rs both call validate_workspace_config before any write, which rejects a default_role that is not a member of allowed_roles. These two functions are the only mutation paths for that geometry (the ConfigEditor typed-setter path never touches allowed_roles/default_role), so the check cannot be bypassed before persistence.

Confirmed still open

• Secret redaction in command failures (real leak, not just a re-check). ShellRunner in crates/jackin-docker/src/shell_runner.rs only redacts -e/--env argv values in the --debug log line (log_command, redact_env_args) and in the tee_to_build_log command string. Every anyhow::bail!("command failed: {} {}", program, args.join(" ")) in run, run_captured, and do_capture joins the raw, unredacted args slice. Launch (crates/jackin-runtime/src/runtime/launch.rs) pushes GITHUB_TOKEN, GH_ENTERPRISE_TOKEN, and other secrets as -e KEY=value pairs into that same args slice, so a failing docker run surfaces the raw token in the CLI/terminal-visible error text. Fix: route every failure-path error string through redact_env_args the same way the debug log path already does.
• Mount validation boundaries — nested sensitive-path and runtime-owned-path gaps. find_sensitive_mounts in crates/jackin-config/src/sensitive.rs only flags a mount whose src exactly ends with a known suffix (/.ssh, /.aws, etc.); mounting a parent directory that merely contains one of those paths (e.g. the whole home directory) triggers no warning. Separately, no check in crates/jackin-config/src/schema.rs or crates/jackin-config/src/app_config/mounts.rs rejects a mount destination that collides with a runtime-owned container path (e.g. /jackin/...). Scoped/global mount persistence and write-time rejection of structurally invalid rows (duplicate destinations, non-shared global isolation) are already covered by AppConfig::validate_effective_mount_destinations and AppConfig::validate_global_mount_rows, called from crates/jackin-console/src/services/config_save.rs — that half of the original bullet is resolved.
• Config write concurrency. ConfigEditor::open/save in crates/jackin-config/src/editor.rs and atomic_write in crates/jackin-config/src/persist.rs still do a read-modify-rename cycle with no file lock or merge check. Two concurrent jackin processes editing config still silently last-writer-wins.
• Runtime-disable env protection. is_reserved in crates/jackin-core/src/env_model.rs checks only a fixed list of exact runtime env names; JACKIN_DISABLE_* is not in RESERVED_RUNTIME_ENV_VARS, and no prefix-based reservation exists anywhere in the reserved-name check. A role manifest can declare an env.JACKIN_DISABLE_* entry today without validate_reserved_env_names rejecting it.
• Subprocess timeout policy. No timeout wrapper exists in ShellRunner (crates/jackin-docker/src/shell_runner.rs) or CommandRunner (crates/jackin-core/src/runner.rs); every run/capture/capture_secret call awaits the child process unbounded.
• Reproducibility and provenance. Still open as a dedicated roadmap item: Reproducibility and Provenance Pinning.

Dedicated items instead of catalog entries

Use the dedicated pages as source of truth where they exist:

• Docker Runtime Hardening Contract
• Reproducibility and Provenance Pinning
• Security Threat Model & Signed Releases

Reviewed: 2026-07-01. This pass re-verified every row against source: two findings (launcher preview parity, role/selector validation) are fixed and dropped; the remaining five are confirmed still open with concrete file/function references above, including a live secret-redaction gap in ShellRunner's failure-path error strings that is more serious than the prior "needs fresh evidence" framing implied.