Skip to content

fix(ssh-executor): forward agent creds, fix terminal host config, restore linux/arm64#1559

Open
iamcobolt wants to merge 12 commits into
kdlbs:mainfrom
iamcobolt:fix/ssh-darwin-followups
Open

fix(ssh-executor): forward agent creds, fix terminal host config, restore linux/arm64#1559
iamcobolt wants to merge 12 commits into
kdlbs:mainfrom
iamcobolt:fix/ssh-darwin-followups

Conversation

@iamcobolt

@iamcobolt iamcobolt commented Jun 30, 2026

Copy link
Copy Markdown

Fixes found while live-validating #1534 (now merged) against real Apple Silicon Macs over Tailscale. Three focused commits, each with a unit test; go build ./... and the affected packages (lifecycle, task/service, launcher) pass against current main.

1. Forward agent credential env to remote SSH agents (blocks Claude tasks)

Env-authenticated agents (claude-acp reads CLAUDE_CODE_OAUTH_TOKEN, not a credentials file) fail on every SSH remote with Authentication required. The SSH executor's CreateInstanceRequest never set Env, so agentctl's CollectAgentEnv(req.Env) had nothing to merge (Docker uses Env: req.Env).

Now SSH executors run the same credential-resolution path used by remote/containerized executors, then forward only a credential allowlist from the resolved request env: CLAUDE_CODE_OAUTH_TOKEN, ANTHROPIC_API_KEY, OPENAI_API_KEY, GEMINI_API_KEY, GOOGLE_API_KEY, GITHUB_TOKEN, and GH_TOKEN.

This deliberately does not fall back to the control-plane process environment. Forwarding arbitrary host env to an SSH target would bypass per-executor credential scoping and could leak local credentials to a remote host. Empty values are skipped so remote-side env is not clobbered with blanks.

2. Project executor SSH config for terminal / workspace-restore

Opening a terminal or restoring a finished SSH task throws ssh executor: host (or host_alias) is required in executor config. GetWorkspaceInfoForSession copied only ExecutorType and dropped exec.Config (ssh_host, fingerprint, user); those keys only survived via a live ExecutorRunning record, so terminal-state / post-restart / post-cleanup sessions had no host config.

Now the service projects a stable SSH executor-config allowlist into workspace metadata as a fallback. Existing live metadata still wins, alias-only executors are preserved, and session-scoped runtime keys such as remote agentctl port/PID/session dir are intentionally excluded so restore cannot reattach to a stale remote instance.

3. Restore linux/arm64 + enforce signed darwin helper

linux/arm64 was dropped vs the predecessor branch (platform gate, build-agentctl-remote, bundle/desktop validators all omitted it), so ARM Linux remotes were rejected. This restores linux/arm64 across the platform gate, build targets, Docker/service bundle packaging, desktop runtime validators, and release scripts.

The PR also makes darwin helper ad-hoc signing explicit and has bundle validation reject an unsigned agentctl-darwin-arm64, which Apple Silicon would otherwise SIGKILL at launch.

Validation

Live end-to-end on darwin/arm64 over Tailscale: signed agentctl uploads, launches, binds, and a full Claude task completes (stop_reason: end_turn) once credentials reach the agent.

Local validation listed in commits includes go build ./..., affected Go package tests, release desktop runtime checks, and changed-file lint fixes from review.

Generated with Claude Code

Review in cubic

iamcobolt and others added 3 commits June 30, 2026 17:43
Follow-ups to kdlbs#1534 (remote macOS SSH hosts):

- Restore linux/arm64 SSH-host support that existed before this change
  (Graviton/ARM Linux remotes). Re-adds the platform-gate case, the
  build-agentctl-linux-arm64 Make target, the release build/copy steps,
  the runtime-bundle required-helper entry, and the desktop runtime
  validators (Rust/shell/mjs).

- Re-add ad-hoc signing of the darwin agentctl helpers in `make`
  (codesign on macOS, rcodesign on Linux/CI; warn if absent). Go already
  signs darwin/arm64 at link time; this also signs darwin/amd64 and
  re-signs uniformly, keeping the chain notarization-ready.

- Enforce the darwin/arm64 helper carries a code signature at
  bundle-validation time (LC_CODE_SIGNATURE scan). An unsigned
  darwin/arm64 binary is SIGKILLed by Apple Silicon (AMFI) on launch, so
  catch it at packaging instead of on every remote launch. darwin/amd64
  is intentionally exempt (Intel does not enforce; Go does not sign it).

Verified: go build ./..., go test on lifecycle/launcher/ssh/metrics,
release-desktop.test.sh, and a live SSH run to a real Apple Silicon Mac
(agentctl uploaded, launched, "HTTP server bound successfully").

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Env-authenticated agents (claude-acp reads CLAUDE_CODE_OAUTH_TOKEN, not a
credentials file) failed on every SSH remote with "Authentication required".
The SSH executor's CreateInstanceRequest never set Env, so agentctl's
CollectAgentEnv(req.Env) had nothing to merge — unlike Docker (Env: req.Env).

Forward a credential allowlist (CLAUDE_CODE_OAUTH_TOKEN, ANTHROPIC_API_KEY,
OPENAI_API_KEY, GEMINI/GOOGLE_API_KEY, GITHUB_TOKEN, GH_TOKEN) into the remote
agent instance, sourced from req.Env first then the control-plane env — never
HOME/PATH/etc., which would break a different remote.

Note: a complete fix should also run credential resolution for SSH (currently
gated to containerized executors via isContainerizedExecutor) so a
profile-configured token populates req.Env directly.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…-restore

Opening a terminal or restoring a finished SSH task failed with "host (or
host_alias) is required in executor config". GetWorkspaceInfoForSession copied
only ExecutorType from the executor record and dropped exec.Config (ssh_host,
fingerprint, user); those keys only survived via a live ExecutorRunning record,
so terminal-state sessions / post-restart / post-cleanup had no host config.

Project the executor record's persistent SSH config into the workspace metadata
as a fallback (non-overwriting, so a live running record still wins).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jun 30, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Adds agentctl-linux-arm64 across build, packaging, runtime validation, and docs. Adds ad-hoc signing checks for agentctl-darwin-arm64, forwards SSH credential env into remote agent creation, and falls back to SSH executor config metadata in workspace info when no running record exists.

Changes

linux/arm64 agentctl support and packaging

Layer / File(s) Summary
Build and packaging propagation
Makefile, .github/workflows/release.yml, Dockerfile, scripts/release/*, apps/desktop/*, docs/specs/ssh-executor/spec.md
Adds agentctl-linux-arm64 to build, bundle, runtime, verification, desktop, and spec paths.
Backend build plumbing
apps/backend/Makefile
Adds the Linux arm64 build target and ad-hoc signing helper for macOS helper builds.

Runtime validation and SSH flow

Layer / File(s) Summary
Darwin helper signing validation
apps/backend/internal/launcher/bundle.go, apps/backend/internal/launcher/bundle_test.go
Marks agentctl-darwin-arm64 as requiring signing and validates Mach-O code signatures before accepting the bundle.
SSH platform support and env forwarding
apps/backend/internal/orchestrator/executor/executor_execute.go, apps/backend/internal/orchestrator/executor/executor_credentials_test.go, apps/backend/internal/agent/runtime/lifecycle/executor_ssh_operations.go, apps/backend/internal/agent/runtime/lifecycle/executor_ssh_connection_test.go
Expands SSH platform support to linux/arm64, updates credential resolution for SSH, and forwards only allowlisted env values into remote agent creation.

Workspace fallback metadata

Layer / File(s) Summary
Workspace metadata fallback from executor config
apps/backend/internal/agent/runtime/lifecycle/executor_backend.go, apps/backend/internal/task/service/service_turns.go, apps/backend/internal/task/service/service_turns_test.go
Filters SSH executor config metadata and merges it into workspace metadata when no running executor record is present.

Estimated code review effort: 4 (Complex) | ~50 minutes

Possibly related PRs

  • kdlbs/kandev#927: Introduced the SSH executor paths that this PR extends with linux/arm64 support and env forwarding.
  • kdlbs/kandev#1519: Modified desktop runtime verifier executable detection that this PR extends for agentctl-linux-arm64.
  • kdlbs/kandev#1534: Updated runtime bundle validation scaffolding that this PR extends with agentctl-darwin-arm64 code-signature checks.

Poem

I hop through builds with tiny feet,
ARM64 and Darwin now both meet.
Signed bundles glow, env keys glide,
The SSH rabbit runs with careful pride.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ⚠️ Warning It covers the required summary and validation, but omits the mandatory checklist and includes forbidden generated-footer text. Add the Checklist section with the required unchecked items, and remove the Claude Code/cubic attribution footer.
✅ Passed checks (3 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed It clearly summarizes the SSH credential forwarding, host config fallback, and linux/arm64 restore work.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@greptile-apps

greptile-apps Bot commented Jun 30, 2026

Copy link
Copy Markdown

Greptile Summary

This PR addresses three SSH executor regressions found during live Apple Silicon validation: credential forwarding for env-authenticated agents, SSH config projection for terminal/workspace-restore paths, and restoring linux/arm64 remote support across build/packaging/validation sites.

  • Credential forwarding: applyContainerCredentials is now called for SSH executors (via executorNeedsResolvedCredentials), and a new sshRemoteAgentEnv function filters a credential allowlist from req.Env — only explicitly resolved keys are forwarded, never the control-plane process environment.
  • SSH config fallback: GetWorkspaceInfoForSession now projects the executor record's stable connection keys into workspace metadata as a non-overwriting fallback via FilterSSHWorkspaceFallbackConfig, fixing the "host (or host_alias) is required" error on terminal-state sessions.
  • linux/arm64 + darwin signing: agentctl-linux-arm64 is wired into all build, packaging, validation, and Dockerfile sites; bundle validation gains a machoHasCodeSignature check that rejects unsigned agentctl-darwin-arm64 at startup.

Confidence Score: 2/5

Not safe to merge: bundle.go uses io.ReadFull without importing io, causing a compile failure in the launcher package.

The new machoHasCodeSignature function calls io.ReadFull but the io package is absent from the import block. This prevents the launcher package from compiling at all. Separately, even if the import is restored, the io.ReadFull error-handling logic incorrectly treats io.ErrUnexpectedEOF as a fatal parse failure, causing three machoHasCodeSignature unit tests to fail since the synthetic test fixtures are 48 bytes. The rest of the PR is well-structured.

apps/backend/internal/launcher/bundle.go requires two fixes: add io to the import block and correct the io.ErrUnexpectedEOF guard in machoHasCodeSignature.

Important Files Changed

Filename Overview
apps/backend/internal/launcher/bundle.go Adds MustBeSigned field and machoHasCodeSignature parser for bundle validation but io is not imported despite calling io.ReadFull causing a compile error; the io.ErrUnexpectedEOF handling logic is also wrong making all macho-signature tests fail.
apps/backend/internal/launcher/bundle_test.go Tests for signature validation and machoHasCodeSignature are correctly written; they fail at runtime due to the io.ErrUnexpectedEOF handling bug in the implementation.
apps/backend/internal/agent/runtime/lifecycle/executor_ssh_operations.go Adds sshRemoteAgentEnv which forwards only a credential allowlist from req.Env and enables linux/arm64 in requireSupportedRemotePlatform; logic is sound.
apps/backend/internal/agent/runtime/lifecycle/executor_backend.go Introduces sshWorkspaceFallbackKeys and FilterSSHWorkspaceFallbackConfig including MetadataKeySSHHostAlias so alias-only executors survive workspace restore.
apps/backend/internal/task/service/service_turns.go Adds mergeExecutorConfigMetadata to project stable SSH connection config into workspace metadata as a non-overwriting fallback when no live ExecutorRunning record exists.
apps/backend/internal/orchestrator/executor/executor_execute.go Adds executorNeedsResolvedCredentials that gates applyContainerCredentials on SSH in addition to containerized executors.
.github/workflows/release.yml Adds linux/arm64 agentctl cross-build step and copies it alongside the other helpers in the release artifact.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Orch as Orchestrator
    participant Exec as executor_execute.go
    participant SSH as executor_ssh_operations.go
    participant Remote as Remote agentctl
    Orch->>Exec: buildLaunchAgentRequest SSH executor
    Note over Exec: executorNeedsResolvedCredentials ssh true
    Exec->>Exec: applyContainerCredentials populates req.Env
    Exec->>SSH: CreateInstance req
    SSH->>SSH: sshRemoteAgentEnv filters credential allowlist from req.Env
    SSH->>Remote: CreateInstanceRequest with Env credentials
    Remote->>Remote: agentctl reads CLAUDE_CODE_OAUTH_TOKEN
    participant WS as GetWorkspaceInfoForSession
    participant DB as Executor record DB
    WS->>DB: GetExecutorByID
    DB-->>WS: exec Type SSH Config ssh_host
    Note over WS: exec.Type == SSH
    WS->>WS: mergeExecutorConfigMetadata via FilterSSHWorkspaceFallbackConfig
    WS-->>Orch: WorkspaceInfo with ssh_host fallback
    participant BV as validateRuntimeBundle
    participant MO as machoHasCodeSignature
    BV->>BV: check agentctl-darwin-arm64 MustBeSigned true
    BV->>MO: machoHasCodeSignature path
    MO->>MO: io.ReadFull parse Mach-O scan LC_CODE_SIGNATURE
    MO-->>BV: signed ok
    alt not ok
        BV-->>Orch: error not parsable Mach-O
    else not signed
        BV-->>Orch: error not code-signed
    end
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant Orch as Orchestrator
    participant Exec as executor_execute.go
    participant SSH as executor_ssh_operations.go
    participant Remote as Remote agentctl
    Orch->>Exec: buildLaunchAgentRequest SSH executor
    Note over Exec: executorNeedsResolvedCredentials ssh true
    Exec->>Exec: applyContainerCredentials populates req.Env
    Exec->>SSH: CreateInstance req
    SSH->>SSH: sshRemoteAgentEnv filters credential allowlist from req.Env
    SSH->>Remote: CreateInstanceRequest with Env credentials
    Remote->>Remote: agentctl reads CLAUDE_CODE_OAUTH_TOKEN
    participant WS as GetWorkspaceInfoForSession
    participant DB as Executor record DB
    WS->>DB: GetExecutorByID
    DB-->>WS: exec Type SSH Config ssh_host
    Note over WS: exec.Type == SSH
    WS->>WS: mergeExecutorConfigMetadata via FilterSSHWorkspaceFallbackConfig
    WS-->>Orch: WorkspaceInfo with ssh_host fallback
    participant BV as validateRuntimeBundle
    participant MO as machoHasCodeSignature
    BV->>BV: check agentctl-darwin-arm64 MustBeSigned true
    BV->>MO: machoHasCodeSignature path
    MO->>MO: io.ReadFull parse Mach-O scan LC_CODE_SIGNATURE
    MO-->>BV: signed ok
    alt not ok
        BV-->>Orch: error not parsable Mach-O
    else not signed
        BV-->>Orch: error not code-signed
    end
Loading

Comments Outside Diff (1)

  1. apps/backend/internal/launcher/bundle.go, line 3-8 (link)

    P0 Missing "io" import — io.ReadFull is called in machoHasCodeSignature but the io package is not in the import block, so go build ./apps/backend/internal/launcher fails to compile. This blocks every code path that touches validateRuntimeBundle, including the bundle-startup check.

Reviews (8): Last reviewed commit: "Merge branch 'main' into fix/ssh-darwin-..." | Re-trigger Greptile

Comment thread apps/backend/internal/task/service/service_turns.go Outdated

@coderabbitai coderabbitai Bot 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.

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@apps/backend/internal/agent/runtime/lifecycle/executor_ssh_operations.go`:
- Around line 647-649: The environment resolution in the SSH executor is
incorrectly falling back to the backend process environment when req.Env omits a
key, which can leak control-plane credentials to remote hosts. Update the
env-building logic in executor_ssh_operations.go around the val == "" branch so
it only forwards values explicitly present in req.Env, or add an explicit opt-in
path before calling os.Getenv(key); keep the change localized to the SSH env
propagation flow and verify the behavior in the executor path that assembles
remote environment variables.

In `@apps/backend/internal/launcher/bundle.go`:
- Around line 57-63: The bundle validation in runtimeBundle should fail closed
when machoHasCodeSignature cannot parse the helper artifact. Update the
MustBeSigned check so that runtimeBundle treats ok=false from
machoHasCodeSignature(path) as an error path, alongside the existing unsigned
case, and return a clear fmt.Errorf mentioning helper.Label and path. Keep the
logic in bundle.go around helper.MustBeSigned and machoHasCodeSignature so
broken or unreadable darwin/arm64 helpers cannot pass validation.

In `@docs/specs/ssh-executor/spec.md`:
- Around line 84-85: Update the later scope sections in the ssh-executor spec to
match the new support matrix by removing linux/arm64 from any “Out of scope” and
“Future scope” mentions. Keep the platform list consistent with the supported
remote platforms and runtime bundle names already described earlier in the
document, so the scope sections no longer contradict the main support matrix.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 8cfb97a5-7019-4aa3-b5f0-feb257197998

📥 Commits

Reviewing files that changed from the base of the PR and between 7893a5e and 38022b8.

📒 Files selected for processing (15)
  • .github/workflows/release.yml
  • apps/backend/Makefile
  • apps/backend/internal/agent/runtime/lifecycle/executor_ssh_connection_test.go
  • apps/backend/internal/agent/runtime/lifecycle/executor_ssh_operations.go
  • apps/backend/internal/launcher/bundle.go
  • apps/backend/internal/launcher/bundle_test.go
  • apps/backend/internal/task/service/service_turns.go
  • apps/backend/internal/task/service/service_turns_test.go
  • apps/desktop/e2e/desktop-launch-smoke.mjs
  • apps/desktop/src-tauri/src/backend.rs
  • docs/specs/ssh-executor/spec.md
  • scripts/release-desktop.test.sh
  • scripts/release/package-bundle.sh
  • scripts/release/prepare-desktop-runtime.sh
  • scripts/release/verify-desktop-runtime.sh

Comment thread apps/backend/internal/agent/runtime/lifecycle/executor_ssh_operations.go Outdated
Comment thread apps/backend/internal/launcher/bundle.go
Comment thread docs/specs/ssh-executor/spec.md

@chatgpt-codex-connector chatgpt-codex-connector Bot 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 38022b8961

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread apps/desktop/src-tauri/src/backend.rs
Comment thread apps/backend/internal/task/service/service_turns.go Outdated
Comment thread apps/backend/internal/task/service/service_turns.go
@carlosflorencio

Copy link
Copy Markdown
Member

Thanks for the PR. A few things I think need fixing before merge:

  • make service-bundle now looks broken: it builds agentctl-linux-arm64, but the root Makefile copy list doesn’t copy it into dist/kandev/bin, while package-bundle.sh now requires it.
  • Dockerfile also doesn’t copy/chmod agentctl-linux-arm64, so the GHCR image would still fail on linux/arm64 SSH hosts.
  • mergeExecutorConfigMetadata filters through FilterPersistentMetadata, but ssh_host_alias isn’t in that allowlist. Alias-only SSH executors will still fail terminal/workspace restore with “host (or host_alias) is required”.
  • The new Makefile signing macro uses POSIX shell syntax even on the Windows Makefile path, where recipes run under cmd.exe.
  • I’d avoid the os.Getenv fallback in sshRemoteAgentEnv; it can forward control-plane/global credentials to arbitrary SSH hosts. Better to resolve SSH credentials explicitly into req.Env.

Small docs cleanup: docs/specs/ssh-executor/spec.md still says remote Linux arm64 is out of scope, and apps/desktop/AGENTS.md still omits the new helper.

@github-actions

Copy link
Copy Markdown
Contributor

Findings

Suggestion (recommended, doesn't block)

  1. sshRemoteAgentEnv silently falls back to control-plane credentials with no logapps/backend/internal/agent/runtime/lifecycle/executor_ssh_operations.go

    • Issue: When req.Env lacks a credential key, the function falls back to os.Getenv — the control-plane process's own env — with no log message. An operator whose SSH agent fails to authenticate has no way to tell whether the token was sourced from a profile-forwarded req.Env or silently pulled from the host environment.
    • Why: Credential-sourcing bugs are notoriously hard to diagnose. A single debug log line (e.g. "forwarding credential from control-plane env") lets operators confirm which path ran without adding operational overhead.
    • Fix: Add a log.Debug (if a logger is available) or at minimum a structured comment call before env[key] = val in the fallback branch. Since this function is package-internal and stateless, the cleanest fix is to return an additional []string of "sourced-from-cp" keys that the caller (createRemoteAgentInstance) can log. Alternatively, thread the logger down.
  2. mergeExecutorConfigMetadata is unconditional across all executor typesapps/backend/internal/task/service/service_turns.go

    • Issue: The PR description says this fixes SSH sessions ("projection of SSH config"), but the call at mergeExecutorConfigMetadata(info, exec.Config) runs for every executor type (Sprites, Docker, local, SSH, etc.). For non-SSH executors exec.Config is typically empty of persistent SSH keys, so today it's a no-op — but nothing prevents a future executor config field from matching a key in persistentMetadataKeys and silently bleeding into workspace metadata for the wrong executor type.
    • Why: Type-guarding the call makes the intent explicit and avoids the risk of non-SSH executors being affected by persistentMetadataKeys changes in the future.
    • Fix:
      if exec.Type == models.ExecutorTypeSSH {
          mergeExecutorConfigMetadata(info, exec.Config)
      }
      Or, if the helper is genuinely executor-agnostic, rename it and update the comment to reflect that.
  3. No GetWorkspaceInfoForSession integration test for the fixed SSH scenarioapps/backend/internal/task/service/service_turns.go

    • Issue: The bug being fixed (terminal on a completed SSH task fails with "host (or host_alias) is required") has a unit test for the helper (TestMergeExecutorConfigMetadata) but no end-to-end test at the service level: create an SSH executor with ssh_host in Config, create a completed session (no ExecutorRunning record), call GetWorkspaceInfoForSession, assert ssh_host appears in the returned metadata.
    • Why: The helper is simple and well-tested, but the call site has a new code path (exec != nil branch, no running record) that isn't exercised by any existing test. Future refactors to GetWorkspaceInfoForSession might silently re-introduce the bug.
    • Fix: Add a TestGetWorkspaceInfoForSession_SSHConfigProjectedWhenNoRunningRecord test. Pattern follows TestGetWorkspaceInfoForSession_BasicFields — create ExecutorTypeSSH with Config: map[string]string{"ssh_host": "host.example.com"}, no UpsertExecutorRunning, assert info.Metadata["ssh_host"] == "host.example.com".

Summary

Severity Count
Blocker 0
Suggestion 3

Verdict: Ready with suggestions — the three fixes are solid and well-tested overall. Suggestions 1 and 3 improve future debuggability; suggestion 2 is a defensive guard. None blocks merge.

Comment thread apps/backend/internal/task/service/service_turns.go Outdated
Comment thread apps/backend/internal/agent/runtime/lifecycle/executor_ssh_operations.go Outdated
Follow-up fixes for carlosflorencio's review of the SSH darwin/arm64 PR.

Packaging (linux/arm64 was half-wired — built but not shipped):
- root Makefile service-bundle: copy agentctl-linux-arm64 into dist/kandev/bin
  (package-bundle.sh already requires it, so `make service-bundle` was broken).
- Dockerfile: COPY + chmod agentctl-linux-arm64 so the GHCR image can serve
  ARM Linux SSH remotes.
- apps/backend/Makefile: define the adhoc_sign_darwin macro per-OS so the
  Windows (cmd.exe) recipe path no longer runs POSIX shell; it's a no-op on
  Windows (can't codesign darwin there anyway).

Terminal / workspace-restore fallback (service_turns.go):
- Project SSH executor config through a dedicated stable-key allowlist
  (FilterSSHWorkspaceFallbackConfig) instead of FilterPersistentMetadata:
  now includes ssh_host_alias (alias-only executors restore correctly) and
  excludes session-scoped runtime keys (remote agentctl port/PID/session dir)
  so a restore can't reattach to a stale remote instance.
- Gate the fallback to ExecutorTypeSSH.

Credential forwarding (executor_ssh_operations.go + orchestrator):
- Drop the os.Getenv fallback in sshRemoteAgentEnv; forward only keys the
  orchestrator explicitly resolved into req.Env, so the control plane's own
  process credentials can't leak to arbitrary SSH hosts.
- Resolve credentials for SSH executors in the orchestrator
  (executorNeedsResolvedCredentials) so env-authenticated agents still work
  via profile env vars / remote_auth_secrets / GITHUB_TOKEN chain.

Bundle validation (bundle.go): fail closed when a MustBeSigned darwin/arm64
helper can't be parsed as a thin Mach-O (read error / truncation / fat), instead
of treating "undeterminable" as signed.

Docs: remove linux/arm64 from the ssh-executor spec Out-of-scope / Future-scope
sections; add agentctl-linux-arm64 to apps/desktop/AGENTS.md runtime layout.

Tests: updated TestSSHRemoteAgentEnv (no control-plane fallback), extended
TestMergeExecutorConfigMetadata (alias + session-scoped exclusion), added a
service-level GetWorkspaceInfoForSession SSH-restore test, a bundle
fail-closed test, and executorNeedsResolvedCredentials coverage.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@iamcobolt

Copy link
Copy Markdown
Author

Thanks so much for the careful review, @carlosflorencio 🙏 — every one of the six points was spot on, and a couple of them (the bundle copy gaps especially) would have bitten real users. Pushed fixes for all of them. Also grateful to @coderabbitai, @greptile-apps, @chatgpt-codex-connector and the CI review bot — their findings overlapped and reinforced yours, and I've credited the specific catches below.

Here's what changed and why:

Packaging — linux/arm64 was half-wired (built but not shipped)

You nailed the root cause: the platform gate and validators knew about linux/arm64, but the two actual copy steps were written around the earlier darwin work and never picked up the new helper. Fixed both, plus the Windows shell issue:

  1. make service-bundle — the root Makefile now copies agentctl-linux-arm64 into dist/kandev/bin, matching what package-bundle.sh already required (so the bundle ships what it validates).
  2. Dockerfile — added the COPY + chmod for agentctl-linux-arm64, so the GHCR image can actually serve ARM Linux SSH remotes.
  3. Windows signing macro — good catch on the cmd.exe path. adhoc_sign_darwin is now defined per-OS inside the existing ifeq ($(OS),Windows_NT) block; the POSIX branch is unchanged and the Windows branch is a no-op (neither codesign nor rcodesign exists there, and darwin/arm64 is already Go-signed at link time). No more POSIX if … fi under cmd.exe.

ssh_host_alias gap + session-key leak (service_turns.go)

Both real — thank you (and @greptile-apps / @chatgpt-codex-connector, who flagged the same two things). mergeExecutorConfigMetadata now filters through a dedicated allowlist, lifecycle.FilterSSHWorkspaceFallbackConfig, instead of FilterPersistentMetadata. It includes ssh_host_alias (so alias-only executors restore instead of throwing host (or host_alias) is required) and deliberately excludes the session-scoped runtime keys (ssh_remote_agentctl_port/_pid, ssh_remote_session_dir, …) so a restore can't reattach to a dead remote agentctl. The call is also gated to ExecutorTypeSSH now, per the CI bot's suggestion.

os.Getenv fallback → resolve credentials into req.Env

You're right, and this is the change I'm most glad you pushed on. sshRemoteAgentEnv now forwards only keys present in req.Env; the process-env fallback is gone. To keep env-authenticated agents working without that fallback, credential resolution (the orchestrator's applyContainerCredentials) now runs for SSH too (executorNeedsResolvedCredentials), so tokens come from the configured profile / remote_auth_secrets / GITHUB_TOKEN chain rather than a blanket forward of the control-plane env. This is exactly the follow-up my own PR note gestured at — glad to have it properly implemented rather than left as a bridge.

⚠️ One behaviour change worth flagging for anyone reading: env-authenticated agents (e.g. claude-acp reading CLAUDE_CODE_OAUTH_TOKEN) on an SSH executor now need the token supplied via the executor profile (a profile env var, or a remote_auth_secret) — it's no longer inherited from the kandev host's environment. That's the intended per-executor scoping, just noting it so no one is surprised by a working setup that relied on the host env.

Bundle validation fails closed

Thanks @coderabbitaivalidateRuntimeBundle now rejects a MustBeSigned darwin/arm64 helper when machoHasCodeSignature returns ok=false (read error / truncation / unexpected or fat layout) instead of treating "undeterminable" as signed. Enforcement stays scoped to darwin/arm64 only, so the linux helpers and Intel darwin are untouched.

Docs

Added agentctl-linux-arm64 to apps/desktop/AGENTS.md (thanks @chatgpt-codex-connector for catching the stale runtime layout). For the ssh-executor spec: I dropped linux/arm64 from Out of scope (the helper exists now) and listed it in the support matrix, but rather than also quietly delete it from Future scope — which would read like I'm declaring my own untested addition "done" — I converted that item into an explicit "hardware-validate linux/arm64 and darwin/amd64" follow-up. Whether it should be promoted to fully in-scope is really your call as maintainers, so I've left it as supported-but-unverified with a tracked task rather than making that call in my own PR.


Two clarifications for reviewers

On the "restore" wording: a small self-correction — digging through history, linux/arm64 was never actually in main before (pre-macOS-support it was an explicit "arm64 lands when the helper is added" TODO), so this is really a new platform addition rather than a restore. Since the PR title becomes the squashed changelog entry, I'd suggest we retitle it something like "add linux/arm64 SSH remote support" for accuracy — happy to do that on the next push. Apologies for the misleading framing on my part.

Validation status: darwin/arm64 is validated live end-to-end (over Tailscale, against real Apple Silicon). linux/arm64 is wired through the full build/upload/launch path and shares the same cross-compiled helper, but I haven't been able to exercise it on real ARM Linux hardware yet — so I've marked it supported-but-unverified in docs/specs/ssh-executor/spec.md rather than claim more than I've tested. Happy to keep it in (it's cheap and low-risk), and happy to drop it to a follow-up PR instead if you'd prefer to keep this one strictly to the darwin fixes — whatever's easiest for you to merge.

go build ./..., go vet, and the affected packages (launcher, lifecycle, task/service, orchestrator/executor) all pass. Thanks again for the thorough look — happy to iterate on anything here.

@chatgpt-codex-connector

Copy link
Copy Markdown

To use Codex here, create an environment for this repo.

@coderabbitai

coderabbitai Bot commented Jul 2, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== bundle.go machoHasCodeSignature / validateRuntimeBundle ==="
fd -a bundle.go apps/backend/internal/launcher | xargs -I{} sed -n '1,200p' {}

echo "=== root Makefile copy list ==="
fd -a "^Makefile$" --max-depth 1 | xargs -I{} grep -n "agentctl" {}

echo "=== Dockerfile ==="
fd -a "^Dockerfile$" --max-depth 2 | xargs -I{} grep -n "agentctl" {}

echo "=== FilterSSHWorkspaceFallbackConfig ==="
rg -n "FilterSSHWorkspaceFallbackConfig|FilterPersistentMetadata|ssh_host_alias" apps/backend/internal --type=go

echo "=== sshRemoteAgentEnv ==="
rg -n "sshRemoteAgentEnv|os.Getenv|executorNeedsResolvedCredentials" apps/backend/internal/agent/runtime/lifecycle/executor_ssh_operations.go apps/backend/internal/orchestrator 2>/dev/null

echo "=== apps/backend/Makefile signing macro ==="
rg -n "adhoc_sign_darwin|Windows_NT" apps/backend/Makefile

Length of output: 11585


@iamcobolt Thanks for the thorough writeup and for looping in the other bots' findings — I spot-checked the six fixes against the current code and they all check out:

  • Bundle signing (fail-closed)validateRuntimeBundle in apps/backend/internal/launcher/bundle.go now returns a distinct "not a parsable thin darwin/arm64 Mach-O; cannot verify its code signature" error when machoHasCodeSignature returns ok=false, separate from the "not code-signed" error when ok=true, signed=false. That's exactly the fail-closed behavior I was after — no more silently treating "undeterminable" as signed.
  • make service-bundle copy gap — root Makefile now copies agentctl-linux-arm64 alongside the other helpers into dist/kandev/bin (lines 267–271), matching what package-bundle.sh requires.
  • DockerfileCOPY bundle/bin/agentctl-linux-arm64 and the corresponding chmod are now present, so the GHCR image ships the arm64 helper.
  • ssh_host_alias gapservice_turns.go now calls the new lifecycle.FilterSSHWorkspaceFallbackConfig instead of FilterPersistentMetadata, and that allowlist in executor_backend.go explicitly includes MetadataKeySSHHostAlias while excluding the session-scoped runtime keys (ssh_remote_agentctl_port/_pid, ssh_remote_session_dir). Good call gating it to ExecutorTypeSSH too.
  • os.Getenv fallback removedsshRemoteAgentEnv in executor_ssh_operations.go now sources only from req.Env, and executorNeedsResolvedCredentials in executor_execute.go (with its own test file) confirms SSH now goes through the credential-resolution path instead of inheriting the control-plane environment. That's the right tradeoff for per-executor scoping.
  • Windows signing macroadhoc_sign_darwin is now defined twice around the ifeq ($(OS),Windows_NT) block in apps/backend/Makefile, so the cmd.exe path no longer runs POSIX if…fi syntax.

Nice work tightening all of these up, especially the credential-forwarding change — routing through applyContainerCredentials/remote_auth_secrets instead of a blanket env forward is the safer design. The behavior-change callout for env-authenticated agents needing profile-scoped tokens is a good thing to flag explicitly for anyone relying on the old implicit host-env inheritance.

On the retitle suggestion ("add linux/arm64 SSH remote support" vs "restore") — that's a call for @carlosflorencio and the maintainers, but it does match what the history search turned up.

(ノ◕ヮ◕)ノ*:・゚✧

@greptile-apps

greptile-apps Bot commented Jul 2, 2026

Copy link
Copy Markdown

This is a thorough and well-reasoned follow-up — the fixes address every point cleanly, and several of them go meaningfully beyond the minimum.

A few specific callouts:

FilterSSHWorkspaceFallbackConfig is the right design. Having a dedicated allowlist for the fallback path, rather than reusing FilterPersistentMetadata, makes the intent explicit and prevents future keys from accidentally bleeding through. Including ssh_host_alias and excluding the session-scoped runtime keys (_port, _pid, _session_dir) is exactly the correct boundary.

Credential resolution for SSH (executorNeedsResolvedCredentials) is a genuine improvement over the bridge approach. The follow-up in the PR description was gesturing at this, and having it properly wired rather than relying on host-env inheritance is the right call — both for correctness and for the auditing story when SSH auth fails.

Fail-closed on machoHasCodeSignature — glad this landed. A broken artifact silently passing validation and then dying on every Apple Silicon host would have been a confusing failure mode.

On the PR title: agree with the self-correction. "Add linux/arm64 SSH remote support" is more accurate than "restore" given the history, and squash commit messages become changelog entries — worth getting right. Happy to see that as a retitle on the next push.

On the linux/arm64 validation status: "supported-but-unverified" with a tracked follow-up is the right framing. The wiring is complete and the helper is the same cross-compiled binary, so the risk is low — but not claiming live end-to-end coverage you don't have is the correct epistemic posture for a changelog-facing PR.

The behavior change note (env-authenticated agents now need tokens via executor profile, not host env) is well-placed and clearly worded. Anyone who hit this in a working setup will know exactly what changed and why.

LGTM overall — the fixes hold up.

Tip: You can customize Greptile's behavior for this repo with .greptile/rules.md and .greptile/config.json.

@cubic-dev-ai cubic-dev-ai Bot 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.

1 issue found and verified against the latest diff

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread apps/backend/internal/orchestrator/task_operations.go Outdated
@coderabbitai

coderabbitai Bot commented Jul 2, 2026

Copy link
Copy Markdown

Caution

Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted.

Error details
{}

…lable ctx

Addresses cubic-dev-ai's P2 on kdlbs#1559. The resume/launch ready-waits are passed
context.WithoutCancel(ctx) so they survive the caller's request timeout, but
that context has no deadline — and waitForSessionReady only checked the 90s
wall-clock budget between poll iterations, so a single blocking GetTaskSession
query could hang past the intended timeout. Derive an internal
context.WithTimeout(ctx, maxWait) and use it for both the poll select and the
GetTaskSession calls, mirroring what waitForAgentPromptReady already does.
waitForAgentPromptReady was already self-bounded and is unchanged.

Test: TestWaitForSessionReady_BoundsQueriesWithNonCancellableContext asserts the
query receives a context carrying a deadline even when the caller passes a
non-cancellable context.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@iamcobolt

Copy link
Copy Markdown
Author

Thanks @cubic-dev-ai — valid catch. Fixed in f4318602.

waitForSessionReady was passed context.WithoutCancel(ctx) (so the wait survives the caller's request timeout during resume/launch), but that context has no deadline, and the 90s budget was only checked between poll iterations — so a single blocking GetTaskSession could hang past it. It now derives an internal context.WithTimeout(ctx, maxWait) and uses that for both the poll select and the GetTaskSession calls, so every query is bounded while still ignoring the caller's cancellation — mirroring what waitForAgentPromptReady already does (that one was already self-bounded, so it's unchanged). Added TestWaitForSessionReady_BoundsQueriesWithNonCancellableContext to guard it.

@cubic-dev-ai

cubic-dev-ai Bot commented Jul 2, 2026

Copy link
Copy Markdown

Thanks @cubic-dev-ai — valid catch. Fixed in f4318602.

waitForSessionReady was passed context.WithoutCancel(ctx) (so the wait survives the caller's request timeout during resume/launch), but that context has no deadline, and the 90s budget was only checked between poll iterations — so a single blocking GetTaskSession could hang past it. It now derives an internal context.WithTimeout(ctx, maxWait) and uses that for both the poll select and the GetTaskSession calls, so every query is bounded while still ignoring the caller's cancellation — mirroring what waitForAgentPromptReady already does (that one was already self-bounded, so it's unchanged). Added TestWaitForSessionReady_BoundsQueriesWithNonCancellableContext to guard it.

@iamcobolt Thanks for the detailed writeup — the fix looks solid.

The key insight you're solving is that context.WithoutCancel preserves request-scoped values but strips the deadline, so the 90s wall-clock check between iterations doesn't protect against a single GetTaskSession query that blocks indefinitely. Deriving readyCtx with context.WithTimeout(ctx, maxWait) and using it for both the poll select and the DB call is exactly the right pattern — it matches what waitForAgentPromptReady already does, and the TestWaitForSessionReady_BoundsQueriesWithNonCancellableContext test (the deadlineRecordingRepo wrapper checking ctx.Deadline()) pins the invariant nicely.

One thing I'd double-check: the ensureSessionRunning path now passes resumeCtx (which is already context.WithoutCancel) into waitForSessionReady and waitForAgentPromptReady. Since waitForSessionReady internally creates its own context.WithTimeout(readyCtx, maxWait), the caller's WithoutCancel is preserved as the parent and the query deadline is bounded — that's correct. Same for startAgentOnPreparedWorkspace passing launchCtx. No regression risk I can see.

All good from my end — thanks for handling this cleanly.

Tip: get faster answers by chatting with cubic’s review copilot in the review UI

…1582)

These resume/launch readiness-context changes were swept into this SSH-executor
PR by accident and are unrelated to it. Reverted to origin/main here; they now
live in their own PR (kdlbs#1582) so this PR stays scoped to the SSH executor
darwin/arm64 + review fixes.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@iamcobolt

Copy link
Copy Markdown
Author

Housekeeping: the orchestrator resume/launch readiness-context change (and the waitForSessionReady query-bounding fix that @cubic-dev-ai flagged above) turned out to be unrelated to this SSH-executor PR — it had been swept in by accident. I've moved it out to its own PR, #1582, and reverted those three internal/orchestrator/ files here in e153c429, so #1559 is now scoped strictly to the SSH executor darwin/arm64 work + the review fixes. The cubic finding is addressed in #1582.

Comment thread apps/backend/internal/agent/runtime/lifecycle/executor_ssh_operations.go Outdated
Comment thread apps/backend/internal/launcher/bundle.go Outdated
@github-actions

github-actions Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Findings

Suggestion (recommended, doesn't block)

  1. Broken comment line in sshRemoteAgentEnvexecutor_ssh_operations.go:640

    • Issue: "// to the control\n// plane's own process environment:" splits the phrase "control plane's" across two comment lines, hurting readability.
    • Fix: Inline comment posted with a one-line suggestion.
  2. machoHasCodeSignature loads the full binarybundle.go:91

    • Issue: os.ReadFile reads the entire agentctl binary (~20–50 MB) to scan only the load-command table (a few hundred bytes in practice).
    • Why: Not a correctness issue — this runs once at startup and terminates early — but it's a large unnecessary allocation for what is essentially a header scan.
    • Fix: A bounded io.ReadFull into a 64 KB buffer would be more than sufficient. Inline comment with a concrete suggestion posted.
  3. MetadataKeySSHHostAlias absent from persistentMetadataKeysexecutor_backend.go:124–165

    • Issue: The PR's own test comment flags this as a "FilterPersistentMetadata gap": ssh_host_alias is included in the new sshWorkspaceFallbackKeys (fixing the terminal/restore path) but is NOT in persistentMetadataKeys. A task continuation that builds its launch metadata via FilterPersistentMetadata would silently drop the alias for alias-only SSH executors.
    • Why this doesn't block: The orchestrator's agent-launch path reads executor config directly from the DB record, not from filtered session metadata, so new sessions aren't affected. Only a code path that relies solely on FilterPersistentMetadata for SSH routing would hit the gap, and the PR correctly addresses the terminal/restore path that was actually broken.
    • Fix: Add MetadataKeySSHHostAlias: true to persistentMetadataKeys in a follow-up if alias-only executor sessions need to survive the FilterPersistentMetadata resume path too.

Summary

Severity Count
Blocker 0
Suggestion 3

Verdict: Ready to merge with suggestions.

The three fixes are focused and correct. sshRemoteAgentEnv reads only from req.Env (never from os.Getenv), which is the right security boundary — the test suite explicitly verifies that control-plane env vars don't leak to SSH targets. The non-overwriting merge in mergeExecutorConfigMetadata correctly lets a live ExecutorRunning record win over the executor-config fallback. The Mach-O signature check in machoHasCodeSignature is logically sound (magic, ncmds, LC_CODE_SIGNATURE walk). Test coverage is thorough across all three changes. The two inline suggestions are non-blocking.

Comment thread apps/backend/internal/launcher/bundle.go Outdated
@github-actions

github-actions Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

OpenCode findings that could not be posted inline

OpenCode model: opencode-go/minimax-m3
Commit: e153c42931e9

Some findings could not be posted inline, so they are preserved here.

Inline placement unavailable

apps/backend/internal/orchestrator/executor/executor_execute.go:840 - applyContainerCredentials name no longer matches its scope

applyContainerCredentials (line 841) is now called for SSH executors in addition to containerized ones, via the executorNeedsResolvedCredentials gate at line 799. Its docstring on line 840 still says "resolves and injects credentials for containerized executors", which is now stale, and the SSH comment in executor_ssh_operations.go:639 cross-references it by the old name. The rename is a small, low-risk change that prevents future readers from assuming the function is Docker/Sprites-only.

Fix: rename to applyRemoteCredentials (or resolveAndInjectRemoteCredentials) and update the docstring + the cross-reference comment in executor_ssh_operations.go.

CI's changed-file golangci-lint flagged: goconst on the credential env-var
literals (constantized as envKey* — several also recur in the Docker/Sprites
paths), dupword in a bundle test assertion ('true true'), and staticcheck S1009
(redundant nil check before len()) in the executor-config merge test.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@coderabbitai coderabbitai Bot 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.

🧹 Nitpick comments (2)
apps/backend/internal/agent/runtime/lifecycle/executor_backend.go (1)

209-231: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Derive the fallback allowlist from the trusted SSH keys to keep the shared connection-routing set in one place; copy trustedExecutorConfigKeys and add MetadataKeySSHWorkdirRoot and MetadataKeySSHShell so restore and launch stay aligned.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/backend/internal/agent/runtime/lifecycle/executor_backend.go` around
lines 209 - 231, Derive the SSH workspace fallback allowlist from the shared
trusted key set instead of duplicating entries in sshWorkspaceFallbackKeys;
reuse trustedExecutorConfigKeys as the base and add only
MetadataKeySSHWorkdirRoot and MetadataKeySSHShell so executor_backend.go keeps
restore and launch routing aligned. Update the sshWorkspaceFallbackKeys
initialization to reference the same connection-routing keys used by
targetFromMetadata and preserve the two workspace-specific keys needed by the
terminal path.
.github/workflows/release.yml (1)

1049-1051: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win

Consider persist-credentials: false on checkout steps that don't need to push.

Several actions/checkout steps touched by this PR (adding the new ref: ${{ needs.prepare.outputs.ref }} line for the restored linux/arm64 flow) leave the default persist-credentials: true, so the ephemeral GITHUB_TOKEN is written to the local git config for the rest of the job (docker builds, npm publish, homebrew tap update). Static analysis (zizmor artipacked) flags this across all these jobs. Since none of these jobs need to git push after checkout, setting persist-credentials: false reduces the blast radius if a build step or dependency is compromised.

🔒 Example fix (repeat per flagged checkout step)
       - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6
         with:
           ref: ${{ needs.prepare.outputs.ref }}
+          persist-credentials: false

Also applies to: 1120-1122, 1233-1235, 1285-1287, 1388-1391, 1494-1496, 1526-1528

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/release.yml around lines 1049 - 1051, The checkout steps
in the release workflow now use a restored ref but still keep the default
credential persistence, which leaves the GITHUB_TOKEN in git config for later
job steps. Update each affected actions/checkout usage in release.yml to set
persist-credentials: false alongside the existing ref: ${{
needs.prepare.outputs.ref }} (and the other flagged checkout blocks) since these
jobs do not need to push after checkout. Focus on the checkout steps in the
restored linux/arm64 flow and the other touched jobs that run docker builds, npm
publish, or homebrew tap updates.

Source: Linters/SAST tools

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In @.github/workflows/release.yml:
- Around line 1049-1051: The checkout steps in the release workflow now use a
restored ref but still keep the default credential persistence, which leaves the
GITHUB_TOKEN in git config for later job steps. Update each affected
actions/checkout usage in release.yml to set persist-credentials: false
alongside the existing ref: ${{ needs.prepare.outputs.ref }} (and the other
flagged checkout blocks) since these jobs do not need to push after checkout.
Focus on the checkout steps in the restored linux/arm64 flow and the other
touched jobs that run docker builds, npm publish, or homebrew tap updates.

In `@apps/backend/internal/agent/runtime/lifecycle/executor_backend.go`:
- Around line 209-231: Derive the SSH workspace fallback allowlist from the
shared trusted key set instead of duplicating entries in
sshWorkspaceFallbackKeys; reuse trustedExecutorConfigKeys as the base and add
only MetadataKeySSHWorkdirRoot and MetadataKeySSHShell so executor_backend.go
keeps restore and launch routing aligned. Update the sshWorkspaceFallbackKeys
initialization to reference the same connection-routing keys used by
targetFromMetadata and preserve the two workspace-specific keys needed by the
terminal path.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 228ae798-ff86-470f-909e-be61dfffe2c5

📥 Commits

Reviewing files that changed from the base of the PR and between 38022b8 and b53b869.

📒 Files selected for processing (15)
  • .github/workflows/release.yml
  • Dockerfile
  • Makefile
  • apps/backend/Makefile
  • apps/backend/internal/agent/runtime/lifecycle/executor_backend.go
  • apps/backend/internal/agent/runtime/lifecycle/executor_ssh_connection_test.go
  • apps/backend/internal/agent/runtime/lifecycle/executor_ssh_operations.go
  • apps/backend/internal/launcher/bundle.go
  • apps/backend/internal/launcher/bundle_test.go
  • apps/backend/internal/orchestrator/executor/executor_credentials_test.go
  • apps/backend/internal/orchestrator/executor/executor_execute.go
  • apps/backend/internal/task/service/service_turns.go
  • apps/backend/internal/task/service/service_turns_test.go
  • apps/desktop/AGENTS.md
  • docs/specs/ssh-executor/spec.md
✅ Files skipped from review due to trivial changes (2)
  • apps/desktop/AGENTS.md
  • docs/specs/ssh-executor/spec.md
🚧 Files skipped from review as they are similar to previous changes (3)
  • apps/backend/internal/launcher/bundle_test.go
  • apps/backend/Makefile
  • apps/backend/internal/launcher/bundle.go

iamcobolt and others added 4 commits July 3, 2026 09:07
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
good memory optimization

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants