Repository navigation
fix: 收敛 SSH 与发布流程的边界问题 - #12
Merged
Merged
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
The updated skip-directory logic relies on absolute path.parts (can incorrectly skip all files/tests depending on checkout path), and the new control-character validation misses DEL (0x7f) in security-sensitive inputs.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR tightens the boundary behavior between the SSH tooling and the release packaging/CI pipeline, focusing on safer parsing/validation, deterministic packaging, and hardened GitHub Actions configuration.
Changes:
- Improve OpenSSH config parsing + alias/port/add argument validation; extend probe argument validation and document
Match execrisk. - Harden release packaging (exclude
dist/, prune output directory, reject repo-escaping symlinks, validate version before cleanup) and add regression tests. - Pin GitHub Actions to specific SHAs, disable credential persistence, tighten permissions, and add concurrency controls.
File summaries
| File | Description |
|---|---|
| tools/run_tests.py | Skip discovery inside common build/cache directories during test discovery. |
| tools/repo_checks.py | Improve file matching (avoid false positives) and support folded frontmatter parsing. |
| tools/package_release.py | Harden packaging against symlinks/output-dir contamination and make cleanup/version handling safer. |
| tests/test_repo_tools.py | Add tests for folded frontmatter, SKILL discovery, version validation, and packaging hardening. |
| ssh/scripts/ssh_probe.py | Add probe argument validation for timeouts and known-hosts file safety. |
| ssh/tests/test_ssh_probe.py | Add tests for probe timeout validation and known-hosts control-character rejection. |
| ssh/scripts/ssh_config.py | Fix host parsing boundaries, improve deterministic resolution, validate inputs, and safer ssh -G invocation. |
| ssh/tests/test_ssh_config.py | Add extensive regression tests for parsing boundaries, add validation, and resolution ranking. |
| ssh/SKILL.md | Document probe safety boundary regarding Match exec. |
| ssh/README.md | Document probe safety boundary regarding Match exec. |
| README.md | Update ssh skill feature list/commands. |
| README.en.md | Update ssh skill feature list/commands (English). |
| .github/workflows/release.yml | Pin action SHAs, disable credential persistence, narrow permissions, add concurrency. |
| .github/workflows/ci.yml | Pin action SHAs, disable credential persistence, add concurrency, clarify job naming. |
Review details
- Files reviewed: 14/14 changed files
- Comments generated: 4
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
185
to
+187
| def validate_single_line_field(name: str, value: object) -> None: | ||
| if isinstance(value, str) and ("\n" in value or "\r" in value): | ||
| raise ValueError(f"{name} must be a single line") | ||
| if isinstance(value, str) and any(ord(character) < 32 for character in value): | ||
| raise ValueError(f"{name} must not contain control characters") |
Comment on lines
+30
to
+39
| def validate_arguments(args: argparse.Namespace) -> None: | ||
| validate_alias(args.alias) | ||
| if args.timeout <= 0: | ||
| raise ValueError("timeout must be greater than zero") | ||
| if args.connect_timeout <= 0: | ||
| raise ValueError("connect-timeout must be greater than zero") | ||
| if args.known_hosts_file and any( | ||
| ord(character) < 32 for character in args.known_hosts_file | ||
| ): | ||
| raise ValueError("known-hosts-file must not contain control characters") |
Comment on lines
+20
to
26
| def matching_files(root: Path, pattern: str) -> list[Path]: | ||
| return sorted( | ||
| path | ||
| for path in root.rglob(f"*{suffix}") | ||
| if not any(part in SKIP_DIRECTORIES for part in path.parts) | ||
| for path in root.rglob(pattern) | ||
| if path.is_file() | ||
| and not any(part in SKIP_DIRECTORIES for part in path.parts) | ||
| ) |
| and any(path.glob("test_*.py")) | ||
| and ".git" not in path.parts | ||
| and "__pycache__" not in path.parts | ||
| and not any(part in SKIP_DIRECTORIES for part in path.parts) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
背景
这是收敛后的单提交版本，替代已关闭的 #11。修改集中处理 SSH 环境映射边界、发布打包正确性以及 CI/Release 安全性。
修改内容
Host *、通配 Host、Match、无缩进选项和空行不再污染具体主机记录。add与 probe 参数，覆盖端口、控制字符、Host 模式、注释截断和超时。dist/、测试和缓存目录；验证
dist、skill 内部、仓库同级和仓库父目录四种输出布局验证通过。