Skip to content

🐛 awsssmsession: return the process-kill error, not nil#8665

Open
tas50 wants to merge 1 commit into
mainfrom
claude/fix-ssm-session-kill-error
Open

🐛 awsssmsession: return the process-kill error, not nil#8665
tas50 wants to merge 1 commit into
mainfrom
claude/fix-ssm-session-kill-error

Conversation

@tas50

@tas50 tas50 commented Jun 23, 2026

Copy link
Copy Markdown
Member

Bug

connection/ssh/awsssmsession/session_manager.go, on session close:

if err != nil {
    return err
}
if processerr != nil {
    return err   // err is nil here — should be `return processerr`
}

When TerminateSession succeeds (err == nil) but the earlier a.process.Kill() failed, the function returns the nil err, hiding the kill failure. A leaked session-manager-plugin subprocess is reported as a clean close.

Fix

return processerr. (Verified by build; this close path has no mock harness.)

🤖 Generated with Claude Code

https://claude.ai/code/session_015U1ocAxfcBhVYi3f9JnyZZ


Generated by Claude Code

@github-actions

github-actions Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@mondoo-code-review mondoo-code-review 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.

Fixes SSM session cleanup silently swallowing process-kill errors by returning the correct variable.

@tas50 tas50 force-pushed the claude/fix-ssm-session-kill-error branch from 0bab3c5 to 936f247 Compare June 23, 2026 17:50
@github-actions

github-actions Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Test Results

11 153 tests  ±0   11 146 ✅ ±0   5m 17s ⏱️ -3s
   545 suites ±0        7 💤 ±0 
    40 files   ±0        0 ❌ ±0 

Results for commit 56aa96c. ± Comparison against base commit a92a575.

♻️ This comment has been updated with latest results.

@tas50 tas50 force-pushed the claude/fix-ssm-session-kill-error branch 3 times, most recently from 4a1b93f to c00b38d Compare June 24, 2026 18:02
On Close, when TerminateSession succeeded (err == nil) but the earlier
process.Kill() failed, the code returned the nil `err` instead of
`processerr`, hiding the kill failure and reporting a clean close while
a session-manager-plugin subprocess may have leaked.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015U1ocAxfcBhVYi3f9JnyZZ
@tas50 tas50 force-pushed the claude/fix-ssm-session-kill-error branch from c00b38d to 56aa96c Compare June 29, 2026 17:23
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.

1 participant