Skip to content

Commit c0a625f

Browse files
authored
fix(security): block path traversal in static analyzer and redact secrets in findings (#59)
The static analyzer's _scan_references_recursive followed file references from SKILL.md without validating that resolved paths stayed within the skill directory. A crafted markdown link like [x](../../etc/passwd) could read arbitrary host files. Additionally, hardcoded secret findings were emitted with full unredacted values in the description, snippet, and metadata fields, leaking raw secrets into scan reports, CI logs, and API responses. Changes: - Add _is_path_traversal() and _is_within_directory() guards to _scan_references_recursive with CRITICAL findings for traversal attempts - Filter traversal paths in _extract_references_from_content (static.py) - Filter traversal paths in _extract_referenced_files and extract_references_from_file (loader.py) - Add _redact_secret() that preserves type-identifying prefixes (AKIA****, sk_live_****, ghp_****, etc.) while masking the secret value - Apply redaction in _create_finding_from_match for HARDCODED_SECRETS - Add 43 new tests covering path traversal prevention and secret redaction
1 parent 5554fd0 commit c0a625f

4 files changed

Lines changed: 669 additions & 10 deletions

File tree

.gitleaksignore

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,3 +16,15 @@ tests/test_policy_knobs.py:stripe-access-token:355
1616
tests/test_policy_knobs.py:stripe-access-token:362
1717
tests/test_policy_knobs.py:stripe-access-token:377
1818
tests/test_policy_knobs.py:stripe-access-token:388
19+
20+
# Fake/synthetic test credentials used in path-traversal and redaction tests.
21+
# These are not real secrets — they exist to verify the redaction logic works.
22+
tests/test_path_traversal_and_redaction.py:stripe-access-token:117
23+
tests/test_path_traversal_and_redaction.py:stripe-access-token:121
24+
tests/test_path_traversal_and_redaction.py:stripe-access-token:316
25+
tests/test_path_traversal_and_redaction.py:stripe-access-token:460
26+
tests/test_path_traversal_and_redaction.py:aws-access-token:111
27+
tests/test_path_traversal_and_redaction.py:aws-access-token:251
28+
tests/test_path_traversal_and_redaction.py:aws-access-token:291
29+
tests/test_path_traversal_and_redaction.py:aws-access-token:440
30+
tests/test_path_traversal_and_redaction.py:aws-access-token:501

skill_scanner/core/analyzers/static.py

Lines changed: 109 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,58 @@
116116
}
117117

118118

119+
def _is_path_traversal(ref_path: str) -> bool:
120+
"""Check if a reference path contains traversal sequences or is absolute."""
121+
return ".." in ref_path or ref_path.startswith("/")
122+
123+
124+
def _is_within_directory(path: Path, directory: Path) -> bool:
125+
"""Check if a resolved path stays within the given directory."""
126+
try:
127+
resolved_path = path.resolve()
128+
resolved_directory = directory.resolve()
129+
return resolved_path.is_relative_to(resolved_directory)
130+
except (ValueError, OSError):
131+
return False
132+
133+
134+
def _redact_secret(text: str) -> str:
135+
"""Redact a matched secret, preserving a short prefix for identification.
136+
137+
Returns a version like ``AKIA****`` or ``sk_live_****`` so the type of
138+
secret is still recognisable in the report without exposing the full value.
139+
"""
140+
if not text:
141+
return text
142+
_KNOWN_PREFIXES = {
143+
"AKIA": 4,
144+
"AGPA": 4,
145+
"AIDA": 4,
146+
"AROA": 4,
147+
"AIPA": 4,
148+
"ANPA": 4,
149+
"ANVA": 4,
150+
"ASIA": 4,
151+
"AIza": 4,
152+
}
153+
for prefix, length in _KNOWN_PREFIXES.items():
154+
if text.startswith(prefix):
155+
return text[:length] + "****"
156+
_TOKEN_PREFIXES = ("sk_live_", "pk_live_", "sk_test_", "pk_test_", "ghp_", "gho_", "ghu_", "ghs_", "ghr_")
157+
for prefix in _TOKEN_PREFIXES:
158+
if text.startswith(prefix):
159+
return prefix + "****"
160+
if text.startswith("eyJ"):
161+
return "eyJ****"
162+
_PK_MARKER_BEGIN = "-----BEGIN"
163+
_PK_MARKER_TYPE = "PRIVATE KEY"
164+
if _PK_MARKER_BEGIN in text and _PK_MARKER_TYPE in text:
165+
return f"{_PK_MARKER_BEGIN} {_PK_MARKER_TYPE}----- [REDACTED]"
166+
if len(text) <= 8:
167+
return text[:2] + "****"
168+
return text[:4] + "****"
169+
170+
119171
class StaticAnalyzer(BaseAnalyzer):
120172
"""Static pattern-based security analyzer."""
121173

@@ -585,6 +637,26 @@ def _scan_references_recursive(
585637
return findings
586638

587639
for ref_file_path in references:
640+
if _is_path_traversal(ref_file_path):
641+
findings.append(
642+
Finding(
643+
id=self._generate_finding_id("PATH_TRAVERSAL", ref_file_path),
644+
rule_id="PATH_TRAVERSAL_ATTEMPT",
645+
category=ThreatCategory.DATA_EXFILTRATION,
646+
severity=Severity.CRITICAL,
647+
title="Path traversal attempt in file reference",
648+
description=(
649+
f"Reference '{ref_file_path}' attempts to escape the skill directory. "
650+
f"This is a path traversal attack that could read sensitive files "
651+
f"from the host system."
652+
),
653+
file_path="SKILL.md",
654+
remediation="Remove path traversal sequences from file references",
655+
analyzer="static",
656+
)
657+
)
658+
continue
659+
588660
full_path = skill.directory / ref_file_path
589661
if not full_path.exists():
590662
alt_paths = [
@@ -601,6 +673,25 @@ def _scan_references_recursive(
601673
if not full_path.exists():
602674
continue
603675

676+
if not _is_within_directory(full_path, skill.directory):
677+
findings.append(
678+
Finding(
679+
id=self._generate_finding_id("PATH_TRAVERSAL_RESOLVED", ref_file_path),
680+
rule_id="PATH_TRAVERSAL_ATTEMPT",
681+
category=ThreatCategory.DATA_EXFILTRATION,
682+
severity=Severity.CRITICAL,
683+
title="File reference resolves outside skill directory",
684+
description=(
685+
f"Reference '{ref_file_path}' resolves to a path outside the skill "
686+
f"directory. This could be a path traversal attack."
687+
),
688+
file_path="SKILL.md",
689+
remediation="Ensure all file references point to files within the skill directory",
690+
analyzer="static",
691+
)
692+
)
693+
continue
694+
604695
dedupe_reference_aliases = self.policy.rule_scoping.dedupe_reference_aliases
605696
# De-duplicate aliases to the same physical file (e.g.
606697
# "cover_art_generator.py" and "scripts/cover_art_generator.py").
@@ -690,17 +781,20 @@ def _extract_references_from_content(self, file_path: Path, content: str) -> lis
690781
markdown_links = _MARKDOWN_LINK_PATTERN.findall(content)
691782
for _, link in markdown_links:
692783
if not link.startswith(("http://", "https://", "ftp://", "#")):
693-
references.append(link)
784+
if not _is_path_traversal(link):
785+
references.append(link)
694786

695787
elif suffix == ".py":
696788
import_patterns = _PYTHON_IMPORT_PATTERN.findall(content)
697789
for imp in import_patterns:
698-
if imp:
790+
if imp and not _is_path_traversal(imp):
699791
references.append(f"{imp}.py")
700792

701793
elif suffix in (".sh", ".bash"):
702794
source_patterns = _BASH_SOURCE_PATTERN.findall(content)
703-
references.extend(source_patterns)
795+
for src in source_patterns:
796+
if not _is_path_traversal(src):
797+
references.append(src)
704798

705799
return references
706800

@@ -1340,21 +1434,30 @@ def _create_finding_from_match(self, rule: SecurityRule, match: dict[str, Any])
13401434
except (ValueError, AttributeError):
13411435
pass
13421436

1437+
matched_text = match.get("matched_text", "N/A")
1438+
snippet = match.get("line_content")
1439+
1440+
if rule.category == ThreatCategory.HARDCODED_SECRETS:
1441+
redacted = _redact_secret(matched_text)
1442+
if snippet and matched_text in snippet:
1443+
snippet = snippet.replace(matched_text, redacted)
1444+
matched_text = redacted
1445+
13431446
return Finding(
13441447
id=self._generate_finding_id(rule.id, f"{match.get('file_path', 'unknown')}:{match.get('line_number', 0)}"),
13451448
rule_id=rule.id,
13461449
category=rule.category,
13471450
severity=rule.severity,
13481451
title=rule.description,
1349-
description=f"Pattern detected: {match.get('matched_text', 'N/A')}",
1452+
description=f"Pattern detected: {matched_text}",
13501453
file_path=match.get("file_path"),
13511454
line_number=match.get("line_number"),
1352-
snippet=match.get("line_content"),
1455+
snippet=snippet,
13531456
remediation=rule.remediation,
13541457
analyzer="static",
13551458
metadata={
13561459
"matched_pattern": match.get("matched_pattern"),
1357-
"matched_text": match.get("matched_text"),
1460+
"matched_text": matched_text,
13581461
"aitech": threat_mapping.get("aitech") if threat_mapping else None,
13591462
"aitech_name": threat_mapping.get("aitech_name") if threat_mapping else None,
13601463
"scanner_category": threat_mapping.get("scanner_category") if threat_mapping else None,

skill_scanner/core/loader.py

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -282,7 +282,8 @@ def _extract_referenced_files(self, instruction_body: str) -> list[str]:
282282
for _, link in markdown_links:
283283
# Filter out URLs, keep relative file paths
284284
if not link.startswith(("http://", "https://", "ftp://", "#")):
285-
references.append(link)
285+
if ".." not in link and not link.startswith("/"):
286+
references.append(link)
286287

287288
# Match "see FILE.md" or "refer to FILE.md" patterns
288289
# Use backticks or quotes to identify actual file references, avoiding false matches like "the.py"
@@ -347,8 +348,8 @@ def _extract_referenced_files(self, instruction_body: str) -> list[str]:
347348
references.append(f"assets/{pattern}")
348349
references.append(f"templates/{pattern}")
349350

350-
# Return unique references
351-
return list(set(references))
351+
# Filter out any references with path traversal sequences
352+
return list({r for r in references if ".." not in r and not r.startswith("/")})
352353

353354
def extract_references_from_file(self, file_path: Path, content: str) -> list[str]:
354355
"""
@@ -410,7 +411,8 @@ def extract_references_from_file(self, file_path: Path, content: str) -> list[st
410411
source_patterns = re.findall(r"(?:source|\.)\s+([A-Za-z0-9_\-./]+\.(?:sh|bash))", content)
411412
references.extend(source_patterns)
412413

413-
return list(set(references))
414+
# Filter out any references with path traversal sequences
415+
return list({r for r in references if ".." not in r and not r.startswith("/")})
414416

415417

416418
def load_skill(

0 commit comments

Comments
 (0)