[high] fix(pdf): implement the advertised extract_urls and extract_embedded

[elhoim]

BLUF

• Priority: high.
• analyze_pdf advertises URL and embedded-file extraction that it never performs. The signature accepts extract_urls and extract_embedded, the docstring documents both, and both are echoed into the audited params — then the result is built with summary["urls"] = [] and summary["embedded_files"] = [], and no code path anywhere populates either.
• A PDF carrying a link to an attacker-controlled host and an embedded evil.exe is reported as having no URLs and no embedded files.
• Because nothing was produced, nothing was indexed either — so search() over the case can never surface them. The evidence is not merely missing from one response; it never enters the case at all.
• Fix: add _extract_pdf_urls and _extract_pdf_embedded_files over a shared _run_pdf_parser helper, wire them to the parameters that already existed, and append their findings to the indexed text.
• Scope: src/mulder/server/tools/documents.py. No new dependency — it uses the same pdf-parser the JavaScript extractor already uses.

The bug

    summary["indicators"] = indicators
    summary["risk_assessment"] = risk
    summary["javascript"] = javascript if extract_javascript else []
    summary["urls"] = []                  # <- never populated
    summary["embedded_files"] = []        # <- never populated

grep -n 'urls\|embedded_files' over the module on main returns exactly these two assignments plus the parameter declarations and the docstring that promises them. There is no producer.

The fix

A shared runner, so the new extractors resolve pdf-parser exactly the way _extract_pdf_javascript already did (bundled Didier Stevens script first, PATH binary second):

def _run_pdf_parser(file_path: Path, *args: str) -> str:
    try:
        proc = subprocess.run(
            _pdf_parser_cmd(file_path, *args),
            capture_output=True, text=True,
            timeout=_PDF_PARSER_TIMEOUT, check=False,
        )
    except (subprocess.TimeoutExpired, OSError):
        return ""
    return proc.stdout

/URI action values are reported as uri_action — the links a reader actually follows — and any other http(s) URL in an object body as object_body. Embedded files come from /Filespec entries, de-duplicated because /F and /UF name the same attachment, and flagged suspicious on an executable suffix.

Both are gated on the parameters that already existed, so extract_urls=False still costs nothing. A missing or timing-out pdf-parser returns "" and both extractors yield [] rather than raising — matching the existing JavaScript extractor's behaviour.

The findings are also appended to index_parts. That part is load-bearing rather than cosmetic: analyze_pdf passes a source, so tool_response returns the compact preview envelope, and the case DB is how an analyst reaches this later.

Verified against the real tool

A minimal PDF with one /URI link action and one /Filespec naming evil.exe, through pdf-parser 0.7.11 and the new extractors:

URLS:
   {'url': 'http://malicious.example.com/payload', 'source': 'uri_action', 'object_id': 5}
EMBEDDED:
   {'filename': 'evil.exe', 'object_id': 7, 'suspicious': True}
BENIGN urls: []
BENIGN embedded: []

The test fixtures are that run's verbatim stdout, so the tests need no installed binary but are grounded in real output.

Deliberately out of scope

• Risk scoring is unchanged. _compute_pdf_risk already derives has_embedded_files from pdfid's /EmbeddedFile indicator. Feeding URL reputation or attachment types into the risk level is a separate judgement call and would belong in its own PR.
• Extracting attachment contents. This lists what is embedded; carving the bytes out is a different feature with its own containment questions.
• JavaScript extraction gaps. _extract_pdf_javascript uses --type /JS --filter, which does not reach JavaScript stored as a literal string in an action dictionary. I could not verify that end-to-end here, so I have deliberately not "fixed" it — flagging it instead.
• The olevba and msodde failure paths — open PRs [high] fix(olevba): stop reporting an unreadable document as macro-free #99 and [high] fix(msodde): report a crashed DDE check instead of "no DDE links found" #101. This PR touches neither; it only adds PDF extraction. Verified conflict-free against both with git merge-tree --write-tree, and against the pdfid-count fix submitted alongside this one.

Verification

• uvx pre-commit run over the changed files → ruff, ruff-format, mypy all pass.
• uv run --locked --extra dev pytest tests/ -q → 862 passed, nothing deselected.
• Discriminating check — the tests drive the public analyze_pdf entry point and stub the wrapped tools at subprocess.run, so they import nothing that this PR adds and fail behaviourally against unmodified code. With documents.py restored to origin/main and the new tests kept, 5 of 7 fail:

FAILED test_a_uri_link_action_reaches_the_analyst - assert 'http://malicious.example.com/payload' in '{"indicators": [], "risk_...
FAILED test_an_embedded_executable_reaches_the_analyst - assert 'evil.exe' in '{"indicators": [], "risk_assessment": {"risk_level": ...
FAILED test_an_embedded_executable_is_flagged_suspicious - assert '"filename": "evil.exe"' in '{"indicators": [], "risk_assessment": {...
FAILED test_the_same_attachment_is_not_reported_twice - AssertionError: assert 0 == 1
FAILED test_a_harmless_attachment_is_reported_but_not_flagged - AssertionError: assert 'notes.txt' in 'PDF Analysis: /tmp/.../invoice.pdf'
5 failed, 2 passed

The two that pass on both are the narrowness guards — a benign PDF yields nothing, and extract_urls=False / extract_embedded=False suppress the work rather than merely blanking the result. A detector that flags ordinary documents is as useless as one that misses malicious ones.

Context

Recovered from closed PR #79, which bundled several unrelated PDF detection gaps into a 284-line diff. This is one of them, resubmitted on its own; the pdfid hex-obfuscation gap is submitted separately, and both are branched independently off main with no stacking. No outcome framework and no shared taxonomy are reintroduced, per the review on #32:

focused fixes for specific tools that currently swallow failures or lose partial-result information would be welcome.

Branched fresh from current main (2e5432c); the closed branch was not revised in place.

🤖 Generated with Claude Code