fix: address PR review round 2 - symlink validation, name checks, stderr

1. Reject symlinks pointing outside scripts/ during discovery (security).
2. Validate skill names against tool-name rules (alphanumeric/hyphens/underscores, max 64 chars).
3. Include stderr in output on successful execution (preserves warnings).
4. Add SKILLS_* vars to .env.example for discoverability.

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

Author: OriNachum

.env.example

@@ -11,6 +11,11 @@ API_ADAPTER_PORT=8080
 STREAM_TIMEOUT=120.0
 HEARTBEAT_INTERVAL=15.0
 
+# Agent Skills Configuration (optional, disabled by default)
+SKILLS_ENABLED=false
+SKILLS_DIR=./skills
+SKILLS_EXEC_TIMEOUT=30
+
 # Logging Configuration (optional)
 LOG_LEVEL=INFO
 LOG_FILE_PATH=./log/api_adapter.log

src/open_responses_server/common/skill_manager.py

@@ -9,9 +9,14 @@
 
 from .config import SKILLS_ENABLED, SKILLS_DIR, SKILLS_EXEC_TIMEOUT, logger
 
+import re
+
 # Maximum characters of SKILL.md instructions to include in tool description
 _MAX_INSTRUCTIONS_LENGTH = 2000
 
+# Tool names must be alphanumeric + hyphens/underscores, max 64 chars
+_VALID_TOOL_NAME_RE = re.compile(r'^[a-zA-Z0-9_-]{1,64}$')
+
 
 def _parse_skill_md(content: str) -> Optional[dict]:
     """Parse SKILL.md content into frontmatter dict + instructions body.
@@ -120,14 +125,35 @@ async def startup_skills(self) -> None:
                     logger.warning(f"[SKILLS-STARTUP] Skipping '{entry.name}': invalid SKILL.md frontmatter")
                     continue
 
+                if not _VALID_TOOL_NAME_RE.match(parsed["name"]):
+                    logger.warning(
+                        f"[SKILLS-STARTUP] Skipping '{entry.name}': "
+                        f"skill name '{parsed['name']}' contains invalid characters "
+                        f"(must be alphanumeric, hyphens, underscores, max 64 chars)"
+                    )
+                    continue
+
                 scripts_dir = entry / "scripts"
                 references_dir = entry / "references"
 
                 available_scripts: List[str] = []
                 if scripts_dir.is_dir():
+                    scripts_dir_resolved = scripts_dir.resolve()
                     for script_file in sorted(scripts_dir.iterdir()):
-                        if script_file.is_file() and os.access(script_file, os.X_OK):
-                            available_scripts.append(script_file.name)
+                        if not script_file.is_file() or not os.access(script_file, os.X_OK):
+                            continue
+                        # Reject symlinks pointing outside scripts/
+                        try:
+                            resolved = script_file.resolve()
+                        except OSError:
+                            continue
+                        if not str(resolved).startswith(str(scripts_dir_resolved) + os.sep):
+                            logger.warning(
+                                f"[SKILLS-STARTUP] Skipping symlink '{script_file.name}' "
+                                f"in skill '{parsed['name']}': points outside scripts/"
+                            )
+                            continue
+                        available_scripts.append(script_file.name)
 
                 if not available_scripts:
                     logger.warning(f"[SKILLS-STARTUP] Skill '{parsed['name']}' has no executable scripts in scripts/")
@@ -289,11 +315,19 @@ async def execute_skill_tool(self, tool_name: str, arguments: dict) -> str:
                 logger.warning(f"[SKILLS-EXEC] {error_msg}")
                 raise RuntimeError(error_msg)
 
+            # Include stderr warnings in output when script succeeds
+            output = stdout_text
+            if stderr_text:
+                if output:
+                    output += "\n\n[stderr]\n" + stderr_text
+                else:
+                    output = stderr_text
+
             logger.info(
                 f"[SKILLS-EXEC] Skill '{skill.name}' script '{script_name}' "
-                f"completed successfully ({len(stdout_text)} chars output)"
+                f"completed successfully ({len(output)} chars output)"
             )
-            return stdout_text if stdout_text else "(no output)"
+            return output if output else "(no output)"
 
         except asyncio.TimeoutError:
             logger.error(