Skip to content

Commit b2e71f5

Browse files
mnriemCopilot
andcommitted
test/fix: address review — robust absolute-path test and tolerant reads
- register_commands(): use is_file() instead of exists() and skip the command if read_text() raises (directory or non-UTF8 file), aligning with the other command/skill readers. - Traversal tests: point the absolute-path payload at the real temp secret.txt (guaranteed to exist on all platforms) instead of /etc/passwd, so the absolute-path guard is genuinely exercised and the test fails if it regresses, rather than passing because the target happens not to exist (e.g. on Windows runners). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 3beeb69 commit b2e71f5

2 files changed

Lines changed: 21 additions & 5 deletions

File tree

‎src/specify_cli/agents.py‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -562,10 +562,13 @@ def register_commands(
562562
except (OSError, ValueError):
563563
continue
564564

565-
if not source_file.exists():
565+
if not source_file.is_file():
566566
continue
567567

568-
content = source_file.read_text(encoding="utf-8")
568+
try:
569+
content = source_file.read_text(encoding="utf-8")
570+
except (OSError, UnicodeDecodeError):
571+
continue
569572
frontmatter, body = self.parse_frontmatter(content)
570573

571574
if frontmatter.get("strategy") == "wrap":

‎tests/test_registrar_path_traversal.py‎

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -135,14 +135,27 @@ def test_rejects_traversal_names(self, tmp_path, bad_name):
135135
_assert_no_stray_files(tmp_path, Path(bad_name).name.replace("/", ""))
136136

137137

138+
ABS_SECRET = "__ABS_SECRET__"
139+
138140
FILE_FIELD_PAYLOADS = [
139141
"../secret.txt",
140142
"../../secret.txt",
141143
"commands/../../secret.txt",
142-
"/etc/passwd",
144+
ABS_SECRET,
143145
]
144146

145147

148+
def _resolve_payload(bad_file: str, secret: Path) -> str:
149+
"""Map the absolute-path sentinel to the real, existing secret file.
150+
151+
Using the temp secret's own absolute path (instead of ``/etc/passwd``)
152+
guarantees the file exists on every platform — so the test fails if the
153+
absolute-path guard regresses, rather than passing because the target
154+
happens not to exist (e.g. on Windows runners).
155+
"""
156+
return str(secret) if bad_file == ABS_SECRET else bad_file
157+
158+
146159
class TestCommandFileTraversal:
147160
"""The manifest ``file`` field must not read host files outside source_dir.
148161
@@ -163,7 +176,7 @@ def test_claude_skips_traversal_in_file_field(self, tmp_path, bad_file):
163176
registrar = CommandRegistrar()
164177
registered = registrar.register_commands(
165178
"claude",
166-
[{"name": "speckit.myext.hello", "file": bad_file, "aliases": []}],
179+
[{"name": "speckit.myext.hello", "file": _resolve_payload(bad_file, secret), "aliases": []}],
167180
"myext",
168181
ext_dir,
169182
project,
@@ -187,7 +200,7 @@ def test_gemini_skips_traversal_in_file_field(self, tmp_path, bad_file):
187200
registrar = CommandRegistrar()
188201
registered = registrar.register_commands(
189202
"gemini",
190-
[{"name": "speckit.myext.hello", "file": bad_file, "aliases": []}],
203+
[{"name": "speckit.myext.hello", "file": _resolve_payload(bad_file, secret), "aliases": []}],
191204
"myext",
192205
ext_dir,
193206
project,

0 commit comments

Comments
 (0)