Skip to content

Commit 4e9c3ad

Browse files
committed
fix: address review feedback from PR #608
- Fix 1: include .cmd and .bat in Windows APM runtimes dir lookup so llm.cmd (installed by setup-llm.ps1) is found when ~/.apm/runtimes is not in PATH - Fix 2: rebase already picked up rust-v0.118.0 pin from origin/main (#663); resolve the setup-codex conflicts in favour of main - Fix 3: add ensure_path_within() checks in _discover_prompt_file and _resolve_prompt_file (PromptCompiler) to catch symlinks that resolve outside the project directory; also filter unsafe dependency matches from rglob results rather than silently including them - Fix runtime mis-detection: reorder _transform_runtime_command to check copilot before codex, preventing "copilot --model codex ..." from being routed to the codex path; replace substring match in _detect_runtime with Path.stem comparison so hyphenated tool names (run-codex-tool) are not mistakenly identified as a known runtime
1 parent 27c3da0 commit 4e9c3ad

File tree

2 files changed

+53
-22
lines changed

2 files changed

+53
-22
lines changed

src/apm_cli/core/script_runner.py

Lines changed: 49 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212

1313
from .token_manager import setup_runtime_environment
1414
from ..output.script_formatters import ScriptExecutionFormatter
15+
from ..utils.path_security import ensure_path_within, PathTraversalError
1516

1617

1718
class ScriptRunner:
@@ -344,38 +345,40 @@ def _transform_runtime_command(
344345
return result
345346

346347
# Handle individual runtime patterns without environment variables
348+
# Note: copilot is checked before codex so that "copilot --model codex ..."
349+
# is not mis-detected as a codex command.
347350

348-
# Handle "codex [args] file.prompt.md [more_args]" -> "codex exec [args] [more_args]"
349-
if re.search(r"codex\s+.*" + re.escape(prompt_file), command):
351+
# Handle "copilot [args] file.prompt.md [more_args]" -> "copilot [args] [more_args]"
352+
if re.search(r"copilot\s+.*" + re.escape(prompt_file), command):
350353
match = re.search(
351-
r"codex\s+(.*?)(" + re.escape(prompt_file) + r")(.*?)$", command
354+
r"copilot\s+(.*?)(" + re.escape(prompt_file) + r")(.*?)$", command
352355
)
353356
if match:
354357
args_before_file = match.group(1).strip()
355358
args_after_file = match.group(3).strip()
356359

357-
result = "codex exec"
360+
result = "copilot"
358361
if args_before_file:
359-
result += f" {args_before_file}"
362+
# Remove any existing -p flag since we'll handle it in execution
363+
cleaned_args = args_before_file.replace("-p", "").strip()
364+
if cleaned_args:
365+
result += f" {cleaned_args}"
360366
if args_after_file:
361367
result += f" {args_after_file}"
362368
return result
363369

364-
# Handle "copilot [args] file.prompt.md [more_args]" -> "copilot [args] [more_args]"
365-
elif re.search(r"copilot\s+.*" + re.escape(prompt_file), command):
370+
# Handle "codex [args] file.prompt.md [more_args]" -> "codex exec [args] [more_args]"
371+
elif re.search(r"codex\s+.*" + re.escape(prompt_file), command):
366372
match = re.search(
367-
r"copilot\s+(.*?)(" + re.escape(prompt_file) + r")(.*?)$", command
373+
r"codex\s+(.*?)(" + re.escape(prompt_file) + r")(.*?)$", command
368374
)
369375
if match:
370376
args_before_file = match.group(1).strip()
371377
args_after_file = match.group(3).strip()
372378

373-
result = "copilot"
379+
result = "codex exec"
374380
if args_before_file:
375-
# Remove any existing -p flag since we'll handle it in execution
376-
cleaned_args = args_before_file.replace("-p", "").strip()
377-
if cleaned_args:
378-
result += f" {cleaned_args}"
381+
result += f" {args_before_file}"
379382
if args_after_file:
380383
result += f" {args_after_file}"
381384
return result
@@ -413,12 +416,19 @@ def _detect_runtime(self, command: str) -> str:
413416
Name of the detected runtime (copilot, codex, llm, or unknown)
414417
"""
415418
command_lower = command.lower().strip()
416-
# Check for runtime keywords anywhere in the command, not just at the start
417-
if "copilot" in command_lower:
419+
if not command_lower:
420+
return "unknown"
421+
# Match on the binary stem only (e.g. "/path/to/codex.exe arg" -> "codex").
422+
# This handles Windows absolute paths being prepended while avoiding false
423+
# positives from tools whose names contain a runtime keyword as a component
424+
# (e.g. "run-codex-tool" must not be detected as codex).
425+
first_arg = command_lower.split()[0]
426+
binary_stem = Path(first_arg).stem
427+
if binary_stem == "copilot":
418428
return "copilot"
419-
elif "codex" in command_lower:
429+
elif binary_stem == "codex":
420430
return "codex"
421-
elif "llm" in command_lower:
431+
elif binary_stem == "llm":
422432
return "llm"
423433
else:
424434
return "unknown"
@@ -506,7 +516,12 @@ def _execute_runtime_command(
506516
exe_name = actual_command_args[0]
507517
apm_runtimes = Path.home() / ".apm" / "runtimes"
508518
# Check APM runtimes directory first
509-
apm_candidates = [apm_runtimes / exe_name, apm_runtimes / f"{exe_name}.exe"]
519+
apm_candidates = [
520+
apm_runtimes / exe_name,
521+
apm_runtimes / f"{exe_name}.exe",
522+
apm_runtimes / f"{exe_name}.cmd",
523+
apm_runtimes / f"{exe_name}.bat",
524+
]
510525
apm_resolved = next((str(c) for c in apm_candidates if c.exists()), None)
511526
if apm_resolved:
512527
actual_command_args[0] = apm_resolved
@@ -558,21 +573,31 @@ def _discover_prompt_file(self, name: str) -> Optional[Path]:
558573

559574
for path in local_search_paths:
560575
if path.exists():
576+
ensure_path_within(path, Path.cwd())
561577
return path
562578

563579
# 2. Search in dependencies and detect collisions
564580
apm_modules = Path("apm_modules")
565581
if apm_modules.exists():
566582
# Collect ALL .prompt.md matches to detect collisions
567-
matches = list(apm_modules.rglob(search_name))
583+
raw_matches = list(apm_modules.rglob(search_name))
568584

569585
# Also search for SKILL.md in directories matching the name
570586
# e.g., name="architecture-blueprint-generator" -> find */architecture-blueprint-generator/SKILL.md
571587
for skill_dir in apm_modules.rglob(name):
572588
if skill_dir.is_dir():
573589
skill_file = skill_dir / "SKILL.md"
574590
if skill_file.exists():
575-
matches.append(skill_file)
591+
raw_matches.append(skill_file)
592+
593+
# Filter out paths that resolve outside the project directory (e.g. malicious symlinks)
594+
matches = []
595+
for m in raw_matches:
596+
try:
597+
ensure_path_within(m, Path.cwd())
598+
matches.append(m)
599+
except PathTraversalError:
600+
pass
576601

577602
if len(matches) == 0:
578603
return None
@@ -1002,13 +1027,15 @@ def _resolve_prompt_file(self, prompt_file: str) -> Path:
10021027

10031028
# First check if it exists in current directory (local)
10041029
if prompt_path.exists():
1030+
ensure_path_within(prompt_path, Path.cwd())
10051031
return prompt_path
10061032

10071033
# Check in common project directories
10081034
common_dirs = [".github/prompts", ".apm/prompts"]
10091035
for common_dir in common_dirs:
10101036
common_path = Path(common_dir) / prompt_file
10111037
if common_path.exists():
1038+
ensure_path_within(common_path, Path.cwd())
10121039
return common_path
10131040

10141041
# If not found locally, search in dependency modules
@@ -1024,12 +1051,14 @@ def _resolve_prompt_file(self, prompt_file: str) -> Path:
10241051
# Check in the root of the repository
10251052
dep_prompt_path = repo_dir / prompt_file
10261053
if dep_prompt_path.exists():
1054+
ensure_path_within(dep_prompt_path, Path.cwd())
10271055
return dep_prompt_path
10281056

10291057
# Also check in common subdirectories
10301058
for subdir in ["prompts", ".", "workflows"]:
10311059
sub_prompt_path = repo_dir / subdir / prompt_file
10321060
if sub_prompt_path.exists():
1061+
ensure_path_within(sub_prompt_path, Path.cwd())
10331062
return sub_prompt_path
10341063

10351064
# If still not found, raise an error with helpful message

tests/unit/test_script_runner.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,8 @@ def test_transform_runtime_command_copilot_with_codex_model_name(self):
162162
@patch('subprocess.run')
163163
@patch('apm_cli.core.script_runner.shutil.which', return_value=None)
164164
@patch('apm_cli.core.script_runner.setup_runtime_environment')
165-
def test_execute_runtime_command_with_env_vars(self, mock_setup_env, mock_which, mock_subprocess):
165+
@patch('apm_cli.core.script_runner.Path.home', return_value=Path('/nonexistent/home'))
166+
def test_execute_runtime_command_with_env_vars(self, mock_home, mock_setup_env, mock_which, mock_subprocess):
166167
"""Test runtime command execution with environment variables."""
167168
mock_setup_env.return_value = {'EXISTING_VAR': 'value'}
168169
mock_subprocess.return_value.returncode = 0
@@ -190,7 +191,8 @@ def test_execute_runtime_command_with_env_vars(self, mock_setup_env, mock_which,
190191
@patch('subprocess.run')
191192
@patch('apm_cli.core.script_runner.shutil.which', return_value=None)
192193
@patch('apm_cli.core.script_runner.setup_runtime_environment')
193-
def test_execute_runtime_command_multiple_env_vars(self, mock_setup_env, mock_which, mock_subprocess):
194+
@patch('apm_cli.core.script_runner.Path.home', return_value=Path('/nonexistent/home'))
195+
def test_execute_runtime_command_multiple_env_vars(self, mock_home, mock_setup_env, mock_which, mock_subprocess):
194196
"""Test runtime command execution with multiple environment variables."""
195197
mock_setup_env.return_value = {}
196198
mock_subprocess.return_value.returncode = 0

0 commit comments

Comments
 (0)