diff --git a/src/apm_cli/bundle/packer.py b/src/apm_cli/bundle/packer.py index 68700e82..1e53cda9 100644 --- a/src/apm_cli/bundle/packer.py +++ b/src/apm_cli/bundle/packer.py @@ -196,12 +196,21 @@ def pack_bundle( # 7. Copy files preserving directory structure for rel_path in unique_files: src = project_root / rel_path + if src.is_symlink(): + continue dest = bundle_dir / rel_path if src.is_dir(): - shutil.copytree(src, dest, dirs_exist_ok=True) + def _ignore_symlinks(directory, entries): + return [ + e for e in entries + if os.path.islink(os.path.join(directory, e)) + ] + shutil.copytree( + src, dest, dirs_exist_ok=True, ignore=_ignore_symlinks, + ) else: dest.parent.mkdir(parents=True, exist_ok=True) - shutil.copy2(src, dest) + shutil.copy2(src, dest, follow_symlinks=False) # 8. Enrich lockfile copy and write to bundle enriched_yaml = enrich_lockfile_for_pack(lockfile, fmt, effective_target) diff --git a/src/apm_cli/bundle/unpacker.py b/src/apm_cli/bundle/unpacker.py index c7b2fd6b..114a9e58 100644 --- a/src/apm_cli/bundle/unpacker.py +++ b/src/apm_cli/bundle/unpacker.py @@ -1,5 +1,6 @@ """Bundle unpacker -- extracts and verifies APM bundles.""" +import os import shutil import sys import tarfile @@ -144,10 +145,24 @@ def unpack_bundle( all_findings = {} for rel_path in unique_files: source_file = source_dir / rel_path - if source_file.is_file() and not source_file.is_symlink(): + if source_file.is_symlink(): + continue + if source_file.is_file(): findings = scanner.scan_file(source_file) if findings: all_findings[rel_path] = findings + elif source_file.is_dir(): + for dirpath, _dirnames, filenames in os.walk( + source_file, followlinks=False + ): + for fname in filenames: + fpath = Path(dirpath) / fname + if fpath.is_symlink(): + continue + findings = scanner.scan_file(fpath) + if findings: + sub_rel = str(fpath.relative_to(source_dir)) + all_findings[sub_rel] = findings if all_findings: flat = [f for ff in all_findings.values() for f in ff] @@ -208,7 +223,14 @@ def unpack_bundle( skipped += 1 continue # skip_verify may allow missing files if src.is_dir(): - shutil.copytree(src, dest, dirs_exist_ok=True, symlinks=True) + def _ignore_symlinks(directory, entries): + return [ + e for e in entries + if os.path.islink(os.path.join(directory, e)) + ] + shutil.copytree( + src, dest, dirs_exist_ok=True, ignore=_ignore_symlinks, + ) else: dest.parent.mkdir(parents=True, exist_ok=True) shutil.copy2(src, dest) diff --git a/src/apm_cli/commands/audit.py b/src/apm_cli/commands/audit.py index 5434c2c8..cf58e79b 100644 --- a/src/apm_cli/commands/audit.py +++ b/src/apm_cli/commands/audit.py @@ -173,7 +173,7 @@ def _render_findings_table( if console: try: from rich.table import Table - from ..security.audit_report import _relative_path + from ..security.audit_report import relative_path table = Table( title=f"{STATUS_SYMBOLS['search']} Content Scan Findings", @@ -194,7 +194,7 @@ def _render_findings_table( for f in rows: table.add_row( f.severity.upper(), - _relative_path(f.file), + relative_path(f.file), f"{f.line}:{f.column}", f.codepoint, f.description, @@ -531,6 +531,13 @@ def audit(ctx, package, file_path, strip, verbose, dry_run, output_format, outpu exit_code = 1 if ContentScanner.has_critical(all_findings) else 2 if effective_format == "text": + if output_path: + _rich_error( + f"Cannot write text format to file. " + f"Use --format json/sarif/markdown with --output, " + f"or rename to a recognized extension (.json, .sarif, .md)" + ) + sys.exit(1) if findings_by_file: _render_findings_table(findings_by_file, verbose=verbose) _render_summary(findings_by_file, files_scanned) diff --git a/src/apm_cli/security/audit_report.py b/src/apm_cli/security/audit_report.py index 5e166aa3..6c9a597d 100644 --- a/src/apm_cli/security/audit_report.py +++ b/src/apm_cli/security/audit_report.py @@ -7,12 +7,12 @@ from .content_scanner import ScanFinding -def _relative_path(file_path: str) -> str: - """Ensure paths in reports are relative (never absolute).""" +def relative_path(file_path: str) -> str: + """Ensure paths in reports are relative with forward slashes (never absolute).""" p = Path(file_path) if p.is_absolute(): try: - return str(p.relative_to(Path.cwd())) + return p.relative_to(Path.cwd()).as_posix() except ValueError: return p.name return file_path.replace("\\", "/") @@ -60,7 +60,7 @@ def findings_to_json( for finding in all_findings: items.append({ "severity": finding.severity, - "file": _relative_path(finding.file), + "file": relative_path(finding.file), "line": finding.line, "column": finding.column, "codepoint": finding.codepoint, @@ -114,7 +114,7 @@ def findings_to_sarif( { "physicalLocation": { "artifactLocation": { - "uri": _relative_path(finding.file), + "uri": relative_path(finding.file), }, "region": { "startLine": finding.line, @@ -226,7 +226,7 @@ def findings_to_markdown( for f in sorted_findings: sev = f.severity.upper() lines.append( - f"| {sev} | `{_relative_path(f.file)}` | {f.line}:{f.column}" + f"| {sev} | `{relative_path(f.file)}` | {f.line}:{f.column}" f" | `{f.codepoint}` | {f.description} |" ) lines.append("") diff --git a/tests/unit/test_audit_command.py b/tests/unit/test_audit_command.py index d3d4ce9c..ca67d212 100644 --- a/tests/unit/test_audit_command.py +++ b/tests/unit/test_audit_command.py @@ -269,6 +269,14 @@ def test_vs_info_shown_with_verbose(self, runner, vs_info_file): result = runner.invoke(audit, ["--file", str(vs_info_file), "--verbose"]) assert "U+FE0F" in result.output + def test_text_format_with_output_errors(self, runner, clean_file): + """--output with text format (or unknown extension) should error.""" + result = runner.invoke( + audit, ["--file", str(clean_file), "-o", "report.txt"] + ) + assert result.exit_code == 1 + assert "Cannot write text format" in result.output + # ── Lockfile mode tests ────────────────────────────────────────── diff --git a/tests/unit/test_packer.py b/tests/unit/test_packer.py index 10e5f254..6bb215b0 100644 --- a/tests/unit/test_packer.py +++ b/tests/unit/test_packer.py @@ -279,7 +279,10 @@ def test_pack_skips_symlinks(self, tmp_path): # Replace link.md with a symlink to the poisoned file (within project) link_file = project / ".github/agents/link.md" link_file.unlink() - os.symlink(poisoned, link_file) + try: + os.symlink(poisoned, link_file) + except OSError: + pytest.skip("symlinks not supported on this platform") out = tmp_path / "build" diff --git a/tests/unit/test_unpack_security.py b/tests/unit/test_unpack_security.py index 3df60ec3..db78ebb3 100644 --- a/tests/unit/test_unpack_security.py +++ b/tests/unit/test_unpack_security.py @@ -153,3 +153,69 @@ def test_unpack_binary_files_skip(self, tmp_path): assert len(result.files) == 2 assert result.security_warnings == 0 assert result.security_critical == 0 + + def test_unpack_scans_directory_contents(self, tmp_path): + """Directory entries have their nested files scanned for hidden chars.""" + bundle = tmp_path / "bundle" / "test-pkg-1.0.0" + bundle.mkdir(parents=True) + + # Create a directory with a poisoned nested file + skills_dir = bundle / ".github" / "agents" + skills_dir.mkdir(parents=True) + (skills_dir / "safe.md").write_text("Safe content\n", encoding="utf-8") + (skills_dir / "poisoned.md").write_text( + "Hidden \U000E0001 tag", encoding="utf-8" + ) + + # Lockfile references the directory + lockfile = LockFile() + dep = LockedDependency( + repo_url="owner/repo", + resolved_commit="abc123", + deployed_files=[".github/agents"], + ) + lockfile.add_dependency(dep) + lockfile.write(bundle / "apm.lock.yaml") + + output = tmp_path / "target" + output.mkdir() + + with pytest.raises(ValueError, match="Blocked.*critical hidden characters"): + unpack_bundle(bundle, output_dir=output) + + def test_unpack_directory_skips_nested_symlinks(self, tmp_path): + """Nested symlinks inside deployed directories must not be deployed.""" + bundle = tmp_path / "bundle" / "test-pkg-1.0.0" + bundle.mkdir(parents=True) + + # Create a directory with a regular file and a nested symlink + skills_dir = bundle / ".github" / "agents" + skills_dir.mkdir(parents=True) + (skills_dir / "real.md").write_text("Real content\n", encoding="utf-8") + + # Create a symlink pointing outside the bundle + outside = tmp_path / "secret.md" + outside.write_text("Secret content\n", encoding="utf-8") + try: + (skills_dir / "linked.md").symlink_to(outside) + except OSError: + pytest.skip("Platform does not support symlinks") + + # Lockfile references the directory + lockfile = LockFile() + dep = LockedDependency( + repo_url="owner/repo", + resolved_commit="abc123", + deployed_files=[".github/agents"], + ) + lockfile.add_dependency(dep) + lockfile.write(bundle / "apm.lock.yaml") + + output = tmp_path / "target" + output.mkdir() + + result = unpack_bundle(bundle, output_dir=output) + + # Real file deployed, symlink filtered out + assert (output / ".github/agents/real.md").exists() + assert not (output / ".github/agents/linked.md").exists()