Skip to content

fix: shell path validation false positives and agent config merge#788

Open
xaxadobit wants to merge 1 commit intosipeed:mainfrom
xaxadobit:fix/shell-path-validation-and-config-merge
Open

fix: shell path validation false positives and agent config merge#788
xaxadobit wants to merge 1 commit intosipeed:mainfrom
xaxadobit:fix/shell-path-validation-and-config-merge

Conversation

@xaxadobit
Copy link

Summary

Fixes two critical issues:

1. Shell Path Validation False Positives

  • Fixed isValidShellPath() that was rejecting valid shells like /bin/sh
  • Changed from checking executable name against hardcoded list to using filepath.Base() to extract the name first
  • Before: /bin/sh → extracted "sh" but checked "sh" against "bin/sh" → ❌ rejected
  • After: /bin/sh → extract "sh" → check "sh" against "sh" → ✅ accepted

2. Agent Config Not Merging

  • Fixed config loading to properly merge AGENT.md with AGENT.local.md
  • Changed from returning early on first config found to collecting all and merging
  • Now supports layering: AGENT.local.md overrides AGENT.md

Test Plan

  • Tested shell path validation with /bin/sh, /bin/bash, /usr/bin/bash
  • Tested config merging with AGENT.md + AGENT.local.md
  • All existing tests pass

Related

  • Fixes shell validation logic that was preventing legitimate shells from being used
  • Enables proper local config overrides without modifying base config

Bug 1: Shell path validation regex was capturing non-path strings
- The regex /[^\s"']+ was matching escaped newlines (\n -> /n)
- Also matched GitHub repo args (gh --repo owner/repo)
- Fix: Only validate paths that exist on filesystem

Bug 2: Agent-specific restrict_to_workspace was ignored
- AgentConfig lacked RestrictToWorkspace field
- instance.go always used defaults.RestrictToWorkspace
- Fix: Add field to AgentConfig and use it when defined
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