Skip to content

Drop poisoned shell-wrapper restore commands #6012

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
lawrencecchen wants to merge 3 commits into
base: main
Choose a base branch
from
+376 −9
Open

Drop poisoned shell-wrapper restore commands #6012

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4b0c609
Add regression coverage for poisoned restore bindings
lawrencecchen Jun 12, 2026
5b59abe
Drop poisoned shell-wrapper restore commands
lawrencecchen Jun 12, 2026
46afbe7
Repair persisted session restore snapshots
lawrencecchen Jun 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 41 additions & 3 deletions Sources/RestorableAgentSession.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -694,7 +694,7 @@ struct SessionRestorableAgentSnapshot: Codable, Sendable {
AgentResumeCommandBuilder.resumeShellCommand(
kind: kind,
sessionId: sessionId,
launchCommand: launchCommand,
launchCommand: trustedLaunchCommandForSessionRestore,
workingDirectory: workingDirectory,
registrationOverride: registration
)
Expand All @@ -704,12 +704,50 @@ struct SessionRestorableAgentSnapshot: Codable, Sendable {
AgentResumeCommandBuilder.forkShellCommand(
kind: kind,
sessionId: sessionId,
launchCommand: launchCommand,
launchCommand: trustedLaunchCommandForSessionRestore,
workingDirectory: workingDirectory,
registrationOverride: registration
)
}

var trustedLaunchCommandForSessionRestore: AgentLaunchCommandSnapshot? {
guard let launchCommand else { return nil }
guard AgentLaunchCaptureTrust.launcherDescribesKind(launchCommand.launcher, kind: kind.rawValue),
!AgentLaunchCaptureTrust.argvLooksLikeShellWrapper(launchCommand.arguments) else {
return nil
}
return launchCommand
}

func repairedForSessionRestore(fallbackWorkingDirectory: String?) -> SessionRestorableAgentSnapshot {
let trustedLaunchCommand = trustedLaunchCommandForSessionRestore
var repaired = self
repaired.launchCommand = trustedLaunchCommand

let fallbackWorkingDirectory = Self.normalizedWorkingDirectory(fallbackWorkingDirectory)
if trustedLaunchCommand == nil {
if repaired.workingDirectory == nil
|| Self.normalizedWorkingDirectory(repaired.workingDirectory)
== Self.normalizedWorkingDirectory(launchCommand?.workingDirectory) {
repaired.workingDirectory = fallbackWorkingDirectory
}
} else if repaired.workingDirectory == nil {
repaired.workingDirectory = Self.normalizedWorkingDirectory(
trustedLaunchCommand?.workingDirectory
) ?? fallbackWorkingDirectory
}
Comment on lines +722 to +738

@coderabbitai coderabbitai Bot Jun 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Preserve .ignore by keeping the repaired snapshot cwd nil.

When trustedLaunchCommandForSessionRestore is nil, this repair path backfills workingDirectory from fallbackWorkingDirectory without checking registration?.cwd == .ignore. That reintroduces a saved cwd into snapshots that are supposed to restore from the caller’s current directory, and downstream restore code in this file explicitly relies on workingDirectory being nil for .ignore agents.

Possible fix 
     let trustedLaunchCommand = trustedLaunchCommandForSessionRestore
     var repaired = self
     repaired.launchCommand = trustedLaunchCommand
+
+    if registration?.cwd == .ignore {
+        repaired.workingDirectory = nil
+        return repaired
+    }
 
     let fallbackWorkingDirectory = Self.normalizedWorkingDirectory(fallbackWorkingDirectory)

Based on learnings: “Registrations with cwd: .ignore set resumeWorkingDirectory to nil, suppressing both the cwd guard in the resume command and the terminal working directory at placement time.”

🤖 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 `@Sources/RestorableAgentSession.swift` around lines 722 - 738, In
repairedForSessionRestore, when trustedLaunchCommandForSessionRestore is nil we
currently backfill repaired.workingDirectory with fallbackWorkingDirectory
unconditionally; change this so we preserve a nil cwd for registrations marked
cwd == .ignore by checking registration?.cwd == .ignore (or the equivalent enum
case) before assigning fallbackWorkingDirectory — if registration?.cwd ==
.ignore leave repaired.workingDirectory as nil; keep existing logic using
Self.normalizedWorkingDirectory(...) and the trustedLaunchCommand branch intact.

Source: Learnings


return repaired
}

private static func normalizedWorkingDirectory(_ workingDirectory: String?) -> String? {
guard let trimmed = workingDirectory?.trimmingCharacters(in: .whitespacesAndNewlines),
!trimmed.isEmpty else {
return nil
}
return ((trimmed as NSString).expandingTildeInPath as NSString).standardizingPath
}

func resumeStartupInput(
fileManager: FileManager = .default,
temporaryDirectory: URL = FileManager.default.temporaryDirectory,
Expand Down Expand Up @@ -741,7 +779,7 @@ struct SessionRestorableAgentSnapshot: Codable, Sendable {
// the current directory (no cd), so the post-exit shell must not force the launch dir.
workingDirectory: registration?.cwd == .ignore
? nil
: (workingDirectory ?? launchCommand?.workingDirectory)
: (workingDirectory ?? trustedLaunchCommandForSessionRestore?.workingDirectory)
) else {
return nil
}
Expand Down
118 changes: 117 additions & 1 deletion Sources/SessionPersistence.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -356,6 +356,24 @@ nonisolated struct SurfaceResumeBindingSnapshot: Codable, Equatable, Sendable {
autoResume == true
}

var trustedForSessionRestore: SurfaceResumeBindingSnapshot? {
isPoisonedAgentHookShellWrapperResume ? nil : self
}

var isPoisonedAgentHookShellWrapperResume: Bool {
guard isAgentHookBinding,
let tokens = SurfaceResumeCommandCanonicalizer.tokens(from: command) else {
return false
}
let commandStart = SurfaceResumeCommandCanonicalizer.commandStartIndexAfterCwdGuard(tokens)
guard commandStart + 1 < tokens.endIndex else { return false }
let executable = (tokens[commandStart] as NSString).lastPathComponent.lowercased()
let shells: Set<String> = ["sh", "bash", "zsh", "dash", "fish", "csh", "tcsh", "ksh"]
guard shells.contains(executable) else { return false }
let resumeWord = tokens[commandStart + 1]
return resumeWord == "resume" || resumeWord == "--resume" || resumeWord.hasPrefix("--resume=")
}

func shouldYieldToDetectedSurfaceResumeBinding(_ detectedBinding: SurfaceResumeBindingSnapshot) -> Bool {
detectedBinding.isProcessDetected && (isProcessDetected || isAgentHookBinding)
}
Expand Down Expand Up @@ -679,6 +697,17 @@ enum SurfaceResumeCommandCanonicalizer {
return ((rawValue as NSString).expandingTildeInPath as NSString).standardizingPath
}

static func commandStartIndexAfterCwdGuard(_ tokens: [String]) -> Int {
guard let first = tokens.first,
first == "{" || first == "cd" else {
return tokens.startIndex
}
guard let andIndex = tokens.firstIndex(of: "&&") else {
return tokens.startIndex
}
return tokens.index(after: andIndex)
Comment on lines +700 to +708

@coderabbitai coderabbitai Bot Jun 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Handle the repo’s || exit $? cwd guard here too.

This helper only skips guards terminated by &&. Sources/SessionRestoredTerminalCommandStore.swift already emits the other in-repo form, { cd -- <cwd> ...; } || exit $? before exec ..., so callers of commandStartIndexAfterCwdGuard will fall back to tokens.startIndex for those commands and parse { as the executable instead of the real resume command. Pattern-match both guard spellings (or the full known guard prefix) here so the canonical start index stays correct everywhere. Based on learnings from the supplied cross-file snippet: Sources/SessionRestoredTerminalCommandStore.swift:3-44 uses a || exit $? cwd guard that this helper does not currently recognize.

🤖 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 `@Sources/SessionPersistence.swift` around lines 700 - 708, The helper
commandStartIndexAfterCwdGuard currently only recognizes a cwd guard terminated
by "&&" and returns tokens.startIndex for the other in-repo form; update it to
also detect the "|| exit $?" guard (or the full known prefix) and treat it the
same as the "&&" case. Concretely, in commandStartIndexAfterCwdGuard(_:), after
confirming tokens.first is "{" or "cd", check for a terminating operator of
either "&&" or "||" (optionally followed by "exit" and "$?") and when found
return tokens.index(after: thatOperatorIndex) so the resume command start is
computed the same for both guard spellings.

}

static func shellQuoted(_ value: String) -> String {
guard !value.isEmpty else { return "''" }
let allowed = CharacterSet(charactersIn: "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_+-=./:@%")
Expand Down Expand Up @@ -1867,6 +1896,89 @@ struct AppSessionSnapshot: Codable, Sendable {
var windows: [SessionWindowSnapshot]
}

enum SessionSnapshotRepairer {
struct Result {
var snapshot: AppSessionSnapshot
var didRepair: Bool
}

static func repair(_ snapshot: AppSessionSnapshot) -> Result {
var didRepair = false
var repaired = snapshot
repaired.windows = repaired.windows.map { window in
repair(window, didRepair: &didRepair)
}
return Result(snapshot: repaired, didRepair: didRepair)
}

private static func repair(
_ window: SessionWindowSnapshot,
didRepair: inout Bool
) -> SessionWindowSnapshot {
var repaired = window
repaired.tabManager.workspaces = repaired.tabManager.workspaces.map { workspace in
repair(workspace, didRepair: &didRepair)
}
return repaired
}

private static func repair(
_ workspace: SessionWorkspaceSnapshot,
didRepair: inout Bool
) -> SessionWorkspaceSnapshot {
var repaired = workspace
repaired.panels = repaired.panels.map { panel in
repair(panel, workspaceDirectory: workspace.currentDirectory, didRepair: &didRepair)
}
return repaired
}

private static func repair(
_ panel: SessionPanelSnapshot,
workspaceDirectory: String,
didRepair: inout Bool
) -> SessionPanelSnapshot {
guard var terminal = panel.terminal else { return panel }
let fallbackWorkingDirectory = firstNormalizedDirectory(
terminal.workingDirectory,
panel.directory,
workspaceDirectory
)

if let resumeBinding = terminal.resumeBinding {
let trustedBinding = resumeBinding.trustedForSessionRestore
if trustedBinding == nil {
didRepair = true
}
terminal.resumeBinding = trustedBinding
}

if let agent = terminal.agent {
let repairedAgent = agent.repairedForSessionRestore(
fallbackWorkingDirectory: fallbackWorkingDirectory
)
if agent.launchCommand != repairedAgent.launchCommand
|| agent.workingDirectory != repairedAgent.workingDirectory {
didRepair = true
}
terminal.agent = repairedAgent
}

var repaired = panel
repaired.terminal = terminal
return repaired
}

private static func firstNormalizedDirectory(_ candidates: String?...) -> String? {
for candidate in candidates {
if let normalized = SurfaceResumeCommandCanonicalizer.normalizedCWD(candidate) {
return normalized
}
}
return nil
}
}

enum SessionPersistenceStore {
enum SnapshotLoadOutcome {
case loaded(AppSessionSnapshot)
Expand All @@ -1885,7 +1997,11 @@ enum SessionPersistenceStore {
guard let snapshot = try? decoder.decode(AppSessionSnapshot.self, from: data) else { return .unusable }
guard snapshot.version == SessionSnapshotSchema.currentVersion else { return .unusable }
guard !snapshot.windows.isEmpty else { return .unusable }
return .loaded(snapshot)
let repairResult = SessionSnapshotRepairer.repair(snapshot)
if repairResult.didRepair {
_ = save(repairResult.snapshot, fileURL: fileURL)
}
return .loaded(repairResult.snapshot)
}

static func load(fileURL: URL? = nil) -> AppSessionSnapshot? {
Expand Down
10 changes: 5 additions & 5 deletions Sources/Workspace.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1101,7 +1101,7 @@ extension Workspace {
) -> String {
let bootstrapCommand = bootstrap.joined(separator: " && ") + " && "
let words = surfaceResumeShellWords(in: command)
let commandStart = hermesAgentCommandStartIndexAfterCwdGuard(words)
let commandStart = surfaceResumeCommandStartIndexAfterCwdGuard(words)
guard commandStart < words.endIndex else {
return bootstrapCommand + command
}
Expand Down Expand Up @@ -1137,7 +1137,7 @@ extension Workspace {

nonisolated private static func hermesAgentCommandByRemovingBootstrapPrefix(_ command: String) -> String {
let words = surfaceResumeShellWords(in: command)
var scanIndex = hermesAgentCommandStartIndexAfterCwdGuard(words)
var scanIndex = surfaceResumeCommandStartIndexAfterCwdGuard(words)
guard scanIndex < words.endIndex else { return command }
let removeStartIndex = scanIndex
var removedBootstrap = false
Expand Down Expand Up @@ -1235,12 +1235,12 @@ extension Workspace {
nonisolated private static func hermesAgentWordsAfterCwdGuard(
_ words: [SurfaceResumeShellWord]
) -> [SurfaceResumeShellWord] {
let commandStart = hermesAgentCommandStartIndexAfterCwdGuard(words)
let commandStart = surfaceResumeCommandStartIndexAfterCwdGuard(words)
guard commandStart < words.endIndex else { return [] }
return Array(words[commandStart...])
}

nonisolated private static func hermesAgentCommandStartIndexAfterCwdGuard(
nonisolated private static func surfaceResumeCommandStartIndexAfterCwdGuard(
_ words: [SurfaceResumeShellWord]
) -> Int {
guard let first = words.first,
Expand Down Expand Up @@ -1637,7 +1637,7 @@ extension Workspace {
) -> UUID? {
switch snapshot.type {
case .terminal:
let resumeBinding = snapshot.terminal?.resumeBinding
let resumeBinding = snapshot.terminal?.resumeBinding?.trustedForSessionRestore
let restorableAgent = snapshot.terminal?.agent
let restoredHibernation = snapshot.terminal?.hibernation
let autoResumeAgentSessions = AgentSessionAutoResumeSettings.isEnabled()
Expand Down
Loading
Loading