From f6ed8e6c977bb29dd45f4b19bc811c7c044e6713 Mon Sep 17 00:00:00 2001 From: zhanghanduo Date: Wed, 26 Aug 2026 08:56:43 +0800 Subject: [PATCH 1/3] fix: resolve CodeQL security alerts --- apodex/clipboard.py | 47 ++++++++-------- apodex/tests/test_clipboard.py | 32 +++++++++-- benchmarks/public/judges/widesearch.py | 2 +- frontier_agent/infra/summary_llm.py | 12 ++-- plugins/tools/_academic_fetch.py | 34 +++++++++-- plugins/tools/_render_check.py | 75 +++++++++++++++++++------ tests/test_code_scanning_regressions.py | 47 ++++++++++++++++ 7 files changed, 193 insertions(+), 56 deletions(-) create mode 100644 tests/test_code_scanning_regressions.py diff --git a/apodex/clipboard.py b/apodex/clipboard.py index 9cfa354..d438d45 100644 --- a/apodex/clipboard.py +++ b/apodex/clipboard.py @@ -12,7 +12,6 @@ import json import os import secrets -import shlex import subprocess import sys import tempfile @@ -118,33 +117,24 @@ def _read_macos_pasteboard(temp_dir: str) -> dict[str, Any]: def _path_text(text: str) -> list[str] | None: - """Return absolute existing paths represented by clipboard text.""" + """Return local paths represented by explicit ``file://`` URLs. + + Plain absolute-path text is deliberately not promoted to an attachment: + copied webpage or chat content must not be able to make the client stage a + readable host file merely because its text happens to name one. + """ raw = text.strip() if not raw: return None lines = [line.strip() for line in raw.splitlines() if line.strip()] - if lines and all(line.startswith("file://") for line in lines): - candidates = [] - for line in lines: - parsed = urlparse(line) - if parsed.scheme == "file": - candidates.append(unquote(parsed.path)) - elif len(lines) > 1 and all( - Path(line.strip("'\"")).expanduser().is_absolute() - and Path(line.strip("'\"")).expanduser().exists() - for line in lines - ): - candidates = [line.strip("'\"") for line in lines] - else: - try: - candidates = shlex.split(raw) - except ValueError: - candidates = [line.strip() for line in raw.splitlines() if line.strip()] - # A single unescaped path may contain spaces. Prefer it when it exists. - if Path(raw).expanduser().is_absolute() and Path(raw).expanduser().exists(): - candidates = [raw] - if not candidates: + if not lines or not all(line.startswith("file://") for line in lines): return None + candidates: list[str] = [] + for line in lines: + parsed = urlparse(line) + if parsed.scheme != "file" or parsed.netloc not in {"", "localhost"}: + return None + candidates.append(unquote(parsed.path)) resolved: list[str] = [] for candidate in candidates: path = Path(candidate).expanduser() @@ -230,8 +220,15 @@ def do_POST(self) -> None: pasted_text = request.get("text") if isinstance(request, dict) else None if pasted_text is not None and not isinstance(pasted_text, str): raise ClipboardError("invalid pasted text") - response = capture_macos_clipboard( - broker.manager, pasted_text=pasted_text, + # The bearer token is available to the container so it can + # call this bridge. Never treat request data as a host path: + # doing so would let container code copy arbitrary readable + # host files into its attachment mount. Only a real macOS + # pasteboard read may produce host file attachments. + response = ( + ClipboardPaste("text", text=pasted_text) + if pasted_text is not None + else capture_macos_clipboard(broker.manager) ) body = json.dumps(asdict(response)).encode() self.send_response(200) diff --git a/apodex/tests/test_clipboard.py b/apodex/tests/test_clipboard.py index 71af796..569a7a8 100644 --- a/apodex/tests/test_clipboard.py +++ b/apodex/tests/test_clipboard.py @@ -22,17 +22,19 @@ def _manager(monkeypatch, tmp_path: Path) -> AttachmentManager: return AttachmentManager(str(tmp_path), "session") -def test_path_text_accepts_quoted_and_file_url_paths(tmp_path: Path) -> None: +def test_path_text_requires_explicit_local_file_urls(tmp_path: Path) -> None: source = tmp_path / "policy wording.pdf" source.write_text("policy") second = tmp_path / "claim photo.png" second.write_bytes(b"png") - assert _path_text(f'"{source}"') == [str(source.resolve())] assert _path_text(source.as_uri()) == [str(source.resolve())] - assert _path_text(f"{source}\n{second}") == [ + assert _path_text(f"{source.as_uri()}\n{second.as_uri()}") == [ str(source.resolve()), str(second.resolve()), ] + assert _path_text(f'"{source}"') is None + assert _path_text(f"{source}\n{second}") is None + assert _path_text("file://evil.test/etc/passwd") is None assert _path_text("ordinary clipboard text") is None @@ -43,7 +45,7 @@ def test_capture_path_text_attaches_instead_of_inserting( source.write_bytes(b"claim") manager = _manager(monkeypatch, tmp_path) - result = capture_macos_clipboard(manager, pasted_text=str(source)) + result = capture_macos_clipboard(manager, pasted_text=source.as_uri()) assert result == ClipboardPaste("attachments", ("claim.pdf",)) assert (manager.staging_dir / "claim.pdf").read_bytes() == b"claim" @@ -88,3 +90,25 @@ def test_broker_round_trip_supports_clipboard_and_pasted_text( assert paste_from_clipboard(manager, pasted_text="pasted").text == "pasted" finally: broker.close() + + +def test_broker_never_resolves_request_text_as_a_host_path( + monkeypatch, tmp_path: Path, +) -> None: + source = tmp_path / "host-secret.txt" + source.write_text("host-only") + manager = _manager(monkeypatch, tmp_path) + try: + broker = ClipboardBroker(manager) + except PermissionError: + pytest.skip("test sandbox does not allow loopback listeners") + broker.start() + monkeypatch.setenv(_BROKER_URL_ENV, f"http://127.0.0.1:{broker.port}") + monkeypatch.setenv(_BROKER_TOKEN_ENV, broker.token) + try: + result = paste_from_clipboard(manager, pasted_text=str(source)) + finally: + broker.close() + + assert result == ClipboardPaste("text", text=str(source)) + assert manager.list() == [] diff --git a/benchmarks/public/judges/widesearch.py b/benchmarks/public/judges/widesearch.py index f430c84..e2cdda5 100644 --- a/benchmarks/public/judges/widesearch.py +++ b/benchmarks/public/judges/widesearch.py @@ -138,7 +138,7 @@ def metric_exact_match(response, target, criterion=None): def metric_url_match(response, target, criterion=None): - pat = re.compile(r"http[s]?://(?:[a-zA-Z]|[0-9]|[$-_@.&+]|[!*\\(\\),]|(?:%[0-9a-fA-F][0-9a-fA-F]))+") + pat = re.compile(r"https?://[^\s<>\"']+") rd = {urlparse(u).netloc for u in pat.findall(response)} td = {urlparse(u).netloc for u in pat.findall(target)} return (1.0, "match") if rd == td else (0.0, "no match") diff --git a/frontier_agent/infra/summary_llm.py b/frontier_agent/infra/summary_llm.py index 1b01230..9b7c0aa 100644 --- a/frontier_agent/infra/summary_llm.py +++ b/frontier_agent/infra/summary_llm.py @@ -3,7 +3,9 @@ import asyncio import hashlib +import hmac import logging +import secrets from contextvars import ContextVar, Token from typing import Any @@ -22,6 +24,7 @@ _MAX_RETRIES = 4 _TRUNCATE_STEP = 40_960 _REQUEST_TIMEOUT = 300 +_KEY_FINGERPRINT_SECRET = secrets.token_bytes(32) # ── Profile-driven override ─────────────────────────────────────────── @@ -186,9 +189,9 @@ def describe_candidates(candidates: list[dict[str, Any]]) -> str: :func:`summary_llm_candidates` returns live API keys, so printing its result verbatim writes them into terminal scrollback, CI logs and pasted bug reports. Debug through this instead: it keeps what identifies a candidate - (provider, model, endpoint) and reduces the key to its length plus a - 12-hex-digit sha256 prefix — enough to tell two keys apart, not enough to - use one. + (provider, model, endpoint) and reduces the key to its length plus a keyed, + process-local 12-hex-digit HMAC prefix — enough to tell two keys apart in + one run without enabling offline guesses from retained logs. """ if not candidates: return "(no summary LLM candidates)" @@ -196,7 +199,8 @@ def describe_candidates(candidates: list[dict[str, Any]]) -> str: for index, cand in enumerate(candidates, 1): key = str(cand.get("api_key") or "") fingerprint = ( - f"len={len(key)} #{hashlib.sha256(key.encode()).hexdigest()[:12]}" + f"len={len(key)} #" + f"{hmac.digest(_KEY_FINGERPRINT_SECRET, key.encode(), hashlib.sha256).hex()[:12]}" if key else "unset" ) lines.append( diff --git a/plugins/tools/_academic_fetch.py b/plugins/tools/_academic_fetch.py index 8ccd330..2d313e5 100644 --- a/plugins/tools/_academic_fetch.py +++ b/plugins/tools/_academic_fetch.py @@ -50,6 +50,13 @@ _ANON_CROSSREF_UA = "FrontierAgent/1.0" +def _is_domain(hostname: str, domain: str) -> bool: + """Return whether *hostname* is *domain* or one of its subdomains.""" + host = hostname.lower().rstrip(".") + allowed = domain.lower().rstrip(".") + return host == allowed or host.endswith("." + allowed) + + # ── URL extraction helpers ──────────────────────────────────────────────── def extract_pmcid(url: str) -> str: @@ -71,12 +78,18 @@ def extract_doi(url: str) -> str: def route_url(url: str) -> Route: """Classify URL for backend selection. Pure — no I/O.""" - domain = urlparse(url).netloc - if "pmc.ncbi.nlm.nih.gov" in domain: + try: + parsed = urlparse(url) + domain = parsed.hostname or "" + except ValueError: + return "jina" + if parsed.scheme.lower() not in {"http", "https"}: + return "jina" + if _is_domain(domain, "pmc.ncbi.nlm.nih.gov"): return "pmc" - if "pubmed.ncbi.nlm.nih.gov" in domain: + if _is_domain(domain, "pubmed.ncbi.nlm.nih.gov"): return "pubmed" - if "biorxiv.org" in domain or "medrxiv.org" in domain: + if _is_domain(domain, "biorxiv.org") or _is_domain(domain, "medrxiv.org"): return "biorxiv" if domain in PAYWALL_DOMAINS: return "paywall" @@ -93,7 +106,18 @@ def is_garbage_content(text: str) -> bool: def biorxiv_to_pdf(url: str) -> str: """Convert bioRxiv/medRxiv URL to full PDF URL. Returns empty if not applicable.""" - if not ("biorxiv.org" in url or "medrxiv.org" in url): + try: + parsed = urlparse(url) + hostname = parsed.hostname or "" + except ValueError: + return "" + if ( + parsed.scheme.lower() not in {"http", "https"} + or not ( + _is_domain(hostname, "biorxiv.org") + or _is_domain(hostname, "medrxiv.org") + ) + ): return "" clean = url.split("?")[0].split("#")[0].rstrip("/") if clean.endswith(".pdf"): diff --git a/plugins/tools/_render_check.py b/plugins/tools/_render_check.py index 05bbba4..044c7d5 100644 --- a/plugins/tools/_render_check.py +++ b/plugins/tools/_render_check.py @@ -13,10 +13,8 @@ r"Warning|Markdown Content):[^\n]*\n+)+", re.IGNORECASE, ) -_HTML_DOC_RE = re.compile( - r"\A(?:\ufeff|\s||<\?xml[^>]*>)*" - r"(?:]*(?:id|class)\s*=\s*['\"][^'\"]*" - r"(?:app|root|__next|__nuxt)[^'\"]*['\"][^>]*>" - r"(?:\s|)*", - re.IGNORECASE, -) - # Body shorter than this counts as "suspiciously empty — worth one retry". # Measured: shell stubs carry 150-170 chars of body, the real page ~4800. MIN_RENDERED_BODY_CHARS = 400 @@ -58,6 +49,8 @@ def __init__(self) -> None: self.hidden_depth = 0 self.all_text: list[str] = [] self.body_text: list[str] = [] + self._mount_stack: list[tuple[str, bool, bool]] = [] + self.has_empty_app_mount = False def handle_starttag( self, @@ -65,6 +58,15 @@ def handle_starttag( attrs: list[tuple[str, str | None]], ) -> None: name = tag.lower() + if self._mount_stack: + mount_tag, candidate, _ = self._mount_stack[-1] + self._mount_stack[-1] = (mount_tag, candidate, True) + attr_map = {key.lower(): value or "" for key, value in attrs} + marker = f"{attr_map.get('id', '')} {attr_map.get('class', '')}".lower() + is_mount = name in {"div", "main", "section"} and any( + token in marker for token in ("app", "root", "__next", "__nuxt") + ) + self._mount_stack.append((name, is_mount, False)) if name == "body": self.seen_body = True self.in_body = True @@ -73,12 +75,19 @@ def handle_starttag( def handle_endtag(self, tag: str) -> None: name = tag.lower() + if self._mount_stack: + mount_tag, candidate, has_content = self._mount_stack.pop() + if mount_tag == name and candidate and not has_content: + self.has_empty_app_mount = True if name in self._HIDDEN and self.hidden_depth: self.hidden_depth -= 1 if name == "body": self.in_body = False def handle_data(self, data: str) -> None: + if data.strip() and self._mount_stack: + mount_tag, candidate, _ = self._mount_stack[-1] + self._mount_stack[-1] = (mount_tag, candidate, True) if self.hidden_depth or not data.strip(): return self.all_text.append(data) @@ -90,14 +99,41 @@ def visible_text(self) -> str: return " ".join(" ".join(parts).split()) -def _visible_body_text(content: str) -> str: +def _parse_visible_body(content: str) -> _VisibleBodyParser: parser = _VisibleBodyParser() try: parser.feed(content) parser.close() except Exception: - return "" - return html.unescape(parser.visible_text()) + pass + return parser + + +def _visible_body_text(content: str) -> str: + return html.unescape(_parse_visible_body(content).visible_text()) + + +def _looks_like_html_document(content: str) -> bool: + """Recognize an HTML document prefix without backtracking over comments.""" + position = 1 if content.startswith("\ufeff") else 0 + length = len(content) + while position < length: + while position < length and content[position].isspace(): + position += 1 + if content.startswith("", position + 4) + if end < 0: + return False + position = end + 3 + continue + if content[position:position + 5].lower() == "", position + 5) + if end < 0: + return False + position = end + 1 + continue + break + return bool(_HTML_START_RE.match(content, position)) def reader_body(content: str) -> str: @@ -122,8 +158,12 @@ def unrendered_kind(content: str) -> str | None: if not text: return "empty" head = text[:4000] - is_html = bool(_HTML_DOC_RE.match(text)) - visible = _visible_body_text(text) if is_html else reader_body(text) + is_html = _looks_like_html_document(text) + parsed_body = _parse_visible_body(text) if is_html else None + visible = ( + html.unescape(parsed_body.visible_text()) + if parsed_body is not None else reader_body(text) + ) if ( is_html and _SCRIPT_TAG_RE.search(head) @@ -134,7 +174,8 @@ def unrendered_kind(content: str) -> str | None: is_html and _SCRIPT_TAG_RE.search(head) and len(visible) < _MAX_SHELL_VISIBLE_BODY_CHARS - and _EMPTY_APP_MOUNT_RE.search(text) + and parsed_body is not None + and parsed_body.has_empty_app_mount ): return "shell" if ( diff --git a/tests/test_code_scanning_regressions.py b/tests/test_code_scanning_regressions.py new file mode 100644 index 0000000..99d02f8 --- /dev/null +++ b/tests/test_code_scanning_regressions.py @@ -0,0 +1,47 @@ +from __future__ import annotations + +from frontier_agent.infra.summary_llm import describe_candidates +from plugins.tools._academic_fetch import biorxiv_to_pdf, route_url +from plugins.tools._render_check import _looks_like_html_document, unrendered_kind + + +def test_academic_routes_validate_the_parsed_hostname() -> None: + assert route_url("https://pmc.ncbi.nlm.nih.gov/articles/PMC1/") == "pmc" + assert route_url("https://subdomain.biorxiv.org/content/1") == "biorxiv" + assert route_url("https://pmc.ncbi.nlm.nih.gov.evil.test/PMC1") == "jina" + assert route_url("https://biorxiv.org@evil.test/content/1") == "jina" + assert route_url("https://[") == "jina" + + +def test_biorxiv_pdf_conversion_rejects_hostname_confusion() -> None: + expected = "https://www.biorxiv.org/content/10.1/123.full.pdf" + assert biorxiv_to_pdf("https://www.biorxiv.org/content/10.1/123") == expected + assert biorxiv_to_pdf("https://biorxiv.org.evil.test/content/10.1/123") == "" + assert biorxiv_to_pdf("https://biorxiv.org@evil.test/content/10.1/123") == "" + assert biorxiv_to_pdf("https://[") == "" + + +def test_render_check_handles_many_leading_comments_without_redos() -> None: + comments = "" * 10_000 + assert _looks_like_html_document(f"{comments}") + + +def test_render_check_still_identifies_an_empty_app_shell() -> None: + comments = "" * 100 + content = f"{comments}
" + assert unrendered_kind(content) == "shell" + + +def test_api_key_fingerprint_is_keyed_and_does_not_expose_the_key() -> None: + candidate = { + "provider": "test", + "model": "model", + "endpoint": "https://example.test/v1", + "api_key": "password-like-low-entropy-value", + } + first = describe_candidates([candidate]) + second = describe_candidates([candidate]) + + assert first == second + assert candidate["api_key"] not in first + assert "len=31 #" in first From a3fb4b93f3c7a775dbb2d0a1e0a6deeb412e5ada Mon Sep 17 00:00:00 2001 From: zhanghanduo Date: Wed, 26 Aug 2026 09:39:58 +0800 Subject: [PATCH 2/3] fix: address code review findings on the CodeQL alert fixes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit _render_check: HTMLParser emits no end tag for a bare void element, so pushing one onto the mount stack leaked a frame and made every later pop close the wrong element — an app mount containing an , or any element closed after a /, could never be flagged as an empty shell. Skip void tags on both sides so the stack stays balanced. Also accept a BOM anywhere in the leading whitespace run again, not only at index 0. clipboard: the broker must not resolve request text as a host path (the container holds the bearer token), while a native paste has no such boundary — the divergence is intentional but was silent, so a Docker user lost drag-to-attach with no explanation. Document the boundary on both entry points, and have the bridge say why the paste stayed text plus the Finder-copy route that does work. Detection is purely textual so the bridge cannot be used as a host-path existence oracle. widesearch: trailing prose punctuation ended up in netloc, so a parenthesized or sentence-final URL never matched its counterpart. Trim it after matching rather than excluding "." from the class, which would truncate every hostname. ci: restore the two gates dropped when frontierchallenge/checks.yml was folded in — no tracked tasks/** payload (check_public_leaks --allow-empty passes a framework-only tree) and registry consistency. check_restricted_software: the path argument was silently ignored; make it a real optional root instead of a comment that lies. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/ci.yml | 20 +++++++++ apodex/clipboard.py | 41 ++++++++++++++++++- apodex/tests/test_clipboard.py | 40 ++++++++++++++++++ apodex/tui/app.py | 4 ++ .../scripts/check_restricted_software.py | 31 ++++++++++---- benchmarks/public/judges/widesearch.py | 22 ++++++++-- plugins/tools/_render_check.py | 23 ++++++++++- tests/test_code_scanning_regressions.py | 29 ++++++++++++- tests/test_file_benchmark_judges.py | 15 +++++++ 9 files changed, 209 insertions(+), 16 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 01ccba5..ee767e8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -59,6 +59,26 @@ jobs: uv run pytest -q \ -c benchmarks/frontierchallenge/pyproject.toml \ benchmarks/frontierchallenge/tests + # check_public_leaks.py --allow-empty passes a framework-only snapshot, + # so the "payload stays on Hugging Face" rule needs its own assertion. + found=$(git ls-files 'benchmarks/frontierchallenge/tasks/**' | head -20) + if [ -n "$found" ]; then + echo "::error::task payload belongs on Hugging Face, not in Git:" + echo "$found" + exit 1 + fi + echo "no tracked task payload" + uv run python - <<'PY' + import json + registry = json.load(open("benchmarks/frontierchallenge/registry.json")) + assert registry["n_tasks"] == len(registry["tasks"]) == 97, ( + f'registry drifted: n_tasks={registry["n_tasks"]}, ' + f'tasks={len(registry["tasks"])}, expected 97' + ) + ids = {row["id"] for row in registry["tasks"]} + assert len(ids) == 97, f"registry has {97 - len(ids)} duplicate task id(s)" + print("public registry: 97 unique task commitments") + PY uv run python benchmarks/frontierchallenge/scripts/check_public_leaks.py \ benchmarks/frontierchallenge --allow-empty uv run python benchmarks/frontierchallenge/scripts/check_restricted_software.py \ diff --git a/apodex/clipboard.py b/apodex/clipboard.py index d438d45..4d7664e 100644 --- a/apodex/clipboard.py +++ b/apodex/clipboard.py @@ -116,12 +116,27 @@ def _read_macos_pasteboard(temp_dir: str) -> dict[str, Any]: return payload +def _looks_like_file_urls(text: str) -> bool: + """Report whether every non-blank line is a ``file://`` URL, without touching disk. + + Used where the text is untrusted and must not be resolved: a purely textual + check leaks nothing about which host paths exist. + """ + lines = [line.strip() for line in text.splitlines() if line.strip()] + return bool(lines) and all(line.startswith("file://") for line in lines) + + def _path_text(text: str) -> list[str] | None: """Return local paths represented by explicit ``file://`` URLs. Plain absolute-path text is deliberately not promoted to an attachment: copied webpage or chat content must not be able to make the client stage a readable host file merely because its text happens to name one. + + Only ever called on input the local user produced — a real pasteboard read, + or a paste into a TUI running natively on the host. Text arriving over the + broker is container-controlled and must not reach this function; see + ``_broker_text_paste``. """ raw = text.strip() if not raw: @@ -147,7 +162,12 @@ def _path_text(text: str) -> list[str] | None: def capture_macos_clipboard( manager: AttachmentManager, *, pasted_text: str | None = None, ) -> ClipboardPaste: - """Capture Finder files, an image, a path string, or ordinary text.""" + """Capture Finder files, an image, a path string, or ordinary text. + + ``pasted_text`` is trusted here: the only callers are the local user's own + paste, either natively or via the broker's pasteboard read. The broker's + request path deliberately does not route request text through this function. + """ if pasted_text is not None: paths = _path_text(pasted_text) if paths is None: @@ -194,6 +214,23 @@ def capture_macos_clipboard( raise ClipboardError(str(payload.get("message") or "unsupported clipboard content")) +def _broker_text_paste(pasted_text: str) -> ClipboardPaste: + """Wrap container-supplied paste text, explaining a gesture the bridge drops. + + Dragging a file into the terminal pastes ``file://`` URLs, which the native + TUI stages as an attachment. Over the bridge the same text is indistinguishable + from a request forged by container code, so it stays text — but the user gets + told why, plus the Finder-copy route that does work through the bridge. + """ + message = ( + "dropped file paths cannot be attached through the container clipboard " + "bridge; copy the file in Finder (Cmd-C) and paste again to attach it" + if _looks_like_file_urls(pasted_text) + else "" + ) + return ClipboardPaste("text", text=pasted_text, message=message) + + class ClipboardBroker: """Loopback-only host service used by the macOS Docker TUI.""" @@ -226,7 +263,7 @@ def do_POST(self) -> None: # host files into its attachment mount. Only a real macOS # pasteboard read may produce host file attachments. response = ( - ClipboardPaste("text", text=pasted_text) + _broker_text_paste(pasted_text) if pasted_text is not None else capture_macos_clipboard(broker.manager) ) diff --git a/apodex/tests/test_clipboard.py b/apodex/tests/test_clipboard.py index 569a7a8..d6f9ffa 100644 --- a/apodex/tests/test_clipboard.py +++ b/apodex/tests/test_clipboard.py @@ -10,6 +10,7 @@ _BROKER_URL_ENV, ClipboardBroker, ClipboardPaste, + _looks_like_file_urls, _path_text, capture_macos_clipboard, paste_from_clipboard, @@ -112,3 +113,42 @@ def test_broker_never_resolves_request_text_as_a_host_path( assert result == ClipboardPaste("text", text=str(source)) assert manager.list() == [] + + +def test_looks_like_file_urls_is_textual_only(tmp_path: Path) -> None: + missing = tmp_path / "absent.pdf" + + # No disk access: a path that does not exist still reads as a file URL, so + # the broker cannot be used as an "does this host file exist" oracle. + assert _looks_like_file_urls(missing.as_uri()) is True + assert _looks_like_file_urls(f"{missing.as_uri()}\n{tmp_path.as_uri()}") is True + assert _looks_like_file_urls(str(missing)) is False + assert _looks_like_file_urls("ordinary clipboard text") is False + assert _looks_like_file_urls("") is False + + +def test_broker_explains_a_dropped_file_instead_of_silently_pasting_text( + monkeypatch, tmp_path: Path, +) -> None: + source = tmp_path / "claim.pdf" + source.write_bytes(b"claim") + manager = _manager(monkeypatch, tmp_path) + try: + broker = ClipboardBroker(manager) + except PermissionError: + pytest.skip("test sandbox does not allow loopback listeners") + broker.start() + monkeypatch.setenv(_BROKER_URL_ENV, f"http://127.0.0.1:{broker.port}") + monkeypatch.setenv(_BROKER_TOKEN_ENV, broker.token) + try: + dropped = paste_from_clipboard(manager, pasted_text=source.as_uri()) + ordinary = paste_from_clipboard(manager, pasted_text="just text") + finally: + broker.close() + + # The same gesture attaches natively, so the bridge says why it could not. + assert dropped.kind == "text" + assert dropped.text == source.as_uri() + assert "Finder" in dropped.message + assert manager.list() == [] + assert ordinary == ClipboardPaste("text", text="just text") diff --git a/apodex/tui/app.py b/apodex/tui/app.py index daf14f9..3f12da6 100644 --- a/apodex/tui/app.py +++ b/apodex/tui/app.py @@ -1027,6 +1027,10 @@ async def _paste_clipboard(self, pasted_text: str | None = None) -> None: ) elif result.kind == "text": self._insert_pasted_text(prompt, result.text) + if result.message: + # e.g. the container bridge cannot attach dropped file paths, so + # the paste stayed text — say why rather than leaving raw URLs. + self.notify(result.message, severity="warning") else: self.notify(result.message or "clipboard is empty or unsupported", severity="warning") self._focus_prompt() diff --git a/benchmarks/frontierchallenge/scripts/check_restricted_software.py b/benchmarks/frontierchallenge/scripts/check_restricted_software.py index bd3087c..72d307b 100644 --- a/benchmarks/frontierchallenge/scripts/check_restricted_software.py +++ b/benchmarks/frontierchallenge/scripts/check_restricted_software.py @@ -9,12 +9,13 @@ """ from __future__ import annotations +import argparse import re import subprocess import sys from pathlib import Path -ROOT = Path(__file__).resolve().parents[1] +DEFAULT_ROOT = Path(__file__).resolve().parents[1] FORBIDDEN_PATHS = { "shared_images/Dockerfile.orca", @@ -35,25 +36,39 @@ ) -def tracked_files() -> list[str]: +def tracked_files(root: Path) -> list[str]: try: out = subprocess.check_output( - ["git", "ls-files", "-z"], cwd=ROOT, stderr=subprocess.DEVNULL + ["git", "ls-files", "-z"], cwd=root, stderr=subprocess.DEVNULL ).decode("utf-8", errors="surrogateescape") return [p for p in out.split("\0") if p] except subprocess.CalledProcessError: # A history-free public export intentionally has no .git directory. # Audit every materialized file there instead of skipping the gate. return sorted( - path.relative_to(ROOT).as_posix() - for path in ROOT.rglob("*") + path.relative_to(root).as_posix() + for path in root.rglob("*") if path.is_file() ) -def main() -> int: +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument( + "root", + nargs="?", + default=DEFAULT_ROOT, + type=Path, + help="tree to audit (default: the FrontierChallenge root beside this script)", + ) + args = parser.parse_args(argv) + root: Path = args.root.resolve() + if not root.is_dir(): + print(f"not a directory: {root}", file=sys.stderr) + return 2 + problems: list[str] = [] - tracked = tracked_files() + tracked = tracked_files(root) for relative in tracked: lower = relative.lower() @@ -64,7 +79,7 @@ def main() -> int: if lower.startswith("shared_images/") and "orca" in Path(lower).name: problems.append(f"ORCA build artifact must not live in shared_images: {relative}") - path = ROOT / relative + path = root / relative if not path.is_file() or path.suffix == ".fcref": continue try: diff --git a/benchmarks/public/judges/widesearch.py b/benchmarks/public/judges/widesearch.py index e2cdda5..7a5696a 100644 --- a/benchmarks/public/judges/widesearch.py +++ b/benchmarks/public/judges/widesearch.py @@ -137,10 +137,26 @@ def metric_exact_match(response, target, criterion=None): return (1.0, "match") if response.lower() == target.lower() else (0.0, "no match") +# Deliberately permissive so IDN/non-ASCII hosts and paths are captured too; +# prose punctuation that a URL cannot end with is trimmed afterwards instead of +# being excluded from the class (a URL body legitimately contains "." and ","). +_URL_RE = re.compile(r"https?://[^\s<>\"']+") +_URL_TRAILING_PUNCTUATION = ".,;:!?'\")]}>" + + +def _url_netlocs(text): + """Collect the hostnames of every URL in ``text``, ignoring prose punctuation.""" + found = set() + for raw in _URL_RE.findall(text): + trimmed = raw.rstrip(_URL_TRAILING_PUNCTUATION) + if trimmed: + found.add(urlparse(trimmed).netloc) + return found + + def metric_url_match(response, target, criterion=None): - pat = re.compile(r"https?://[^\s<>\"']+") - rd = {urlparse(u).netloc for u in pat.findall(response)} - td = {urlparse(u).netloc for u in pat.findall(target)} + rd = _url_netlocs(response) + td = _url_netlocs(target) return (1.0, "match") if rd == td else (0.0, "no match") diff --git a/plugins/tools/_render_check.py b/plugins/tools/_render_check.py index 044c7d5..54f9868 100644 --- a/plugins/tools/_render_check.py +++ b/plugins/tools/_render_check.py @@ -41,6 +41,12 @@ class _VisibleBodyParser(HTMLParser): """Collect visible body text while ignoring script/style/template payloads.""" _HIDDEN = frozenset({"script", "style", "template", "noscript", "svg"}) + # HTMLParser never emits handle_endtag for a bare void element, so pushing + # one onto the mount stack would leak a frame and desync every later pop. + _VOID = frozenset({ + "area", "base", "br", "col", "embed", "hr", "img", "input", + "link", "meta", "param", "source", "track", "wbr", + }) def __init__(self) -> None: super().__init__(convert_charrefs=True) @@ -61,6 +67,10 @@ def handle_starttag( if self._mount_stack: mount_tag, candidate, _ = self._mount_stack[-1] self._mount_stack[-1] = (mount_tag, candidate, True) + if name in self._VOID: + # Already recorded as content for the enclosing mount above; a void + # element has no children and never closes, so it gets no frame. + return attr_map = {key.lower(): value or "" for key, value in attrs} marker = f"{attr_map.get('id', '')} {attr_map.get('class', '')}".lower() is_mount = name in {"div", "main", "section"} and any( @@ -75,6 +85,11 @@ def handle_starttag( def handle_endtag(self, tag: str) -> None: name = tag.lower() + if name in self._VOID: + # Balances handle_starttag: no frame was pushed, so pop nothing. + # Covers both a stray ``
`` and the endtag half that + # handle_startendtag synthesizes for ````. + return if self._mount_stack: mount_tag, candidate, has_content = self._mount_stack.pop() if mount_tag == name and candidate and not has_content: @@ -115,10 +130,14 @@ def _visible_body_text(content: str) -> str: def _looks_like_html_document(content: str) -> bool: """Recognize an HTML document prefix without backtracking over comments.""" - position = 1 if content.startswith("\ufeff") else 0 + position = 0 length = len(content) while position < length: - while position < length and content[position].isspace(): + # A BOM may repeat or trail whitespace when documents are concatenated, + # so skip it wherever it appears in the leading run, not just at index 0. + while position < length and ( + content[position].isspace() or content[position] == "\ufeff" + ): position += 1 if content.startswith("", position + 4) diff --git a/tests/test_code_scanning_regressions.py b/tests/test_code_scanning_regressions.py index 99d02f8..57edf47 100644 --- a/tests/test_code_scanning_regressions.py +++ b/tests/test_code_scanning_regressions.py @@ -2,7 +2,11 @@ from frontier_agent.infra.summary_llm import describe_candidates from plugins.tools._academic_fetch import biorxiv_to_pdf, route_url -from plugins.tools._render_check import _looks_like_html_document, unrendered_kind +from plugins.tools._render_check import ( + _looks_like_html_document, + _parse_visible_body, + unrendered_kind, +) def test_academic_routes_validate_the_parsed_hostname() -> None: @@ -32,6 +36,29 @@ def test_render_check_still_identifies_an_empty_app_shell() -> None: assert unrendered_kind(content) == "shell" +def test_render_check_void_elements_do_not_desync_the_mount_stack() -> None: + # HTMLParser emits no end tag for a bare /, so each one used to + # leak a stack frame and make every later pop close the wrong element. + content = ( + "" + "
" + ) + parser = _parse_visible_body(content) + assert parser._mount_stack == [] + assert parser.has_empty_app_mount is True + assert unrendered_kind(content) == "shell" + + # A self-closing void tag must stay balanced too. + assert _parse_visible_body("
")._mount_stack == [] + + +def test_render_check_accepts_a_bom_anywhere_in_the_leading_run() -> None: + assert _looks_like_html_document("\ufeff\ufeff") + assert _looks_like_html_document("\n\ufeff") + assert _looks_like_html_document("\ufeff ") + assert not _looks_like_html_document("\ufeffplain text") + + def test_api_key_fingerprint_is_keyed_and_does_not_expose_the_key() -> None: candidate = { "provider": "test", diff --git a/tests/test_file_benchmark_judges.py b/tests/test_file_benchmark_judges.py index 29bca6b..0e15fda 100644 --- a/tests/test_file_benchmark_judges.py +++ b/tests/test_file_benchmark_judges.py @@ -37,6 +37,21 @@ def test_gdpval_requires_expected_deliverable_extension(tmp_path: Path) -> None: assert score_gdpval_outputs(tmp_path, target) == (0, 0.0) +def test_widesearch_url_match_ignores_trailing_prose_punctuation() -> None: + pytest.importorskip("pandas") # widesearch pulls the eval extras in at import + from benchmarks.public.judges.widesearch import _url_netlocs, metric_url_match + + # A parenthesized or sentence-final URL used to carry ")"/"." into the netloc + # and so never matched its counterpart. + assert _url_netlocs("see (https://example.com)") == {"example.com"} + assert _url_netlocs("source: https://example.com.") == {"example.com"} + assert metric_url_match("see (https://example.com).", "https://example.com")[0] == 1.0 + + # Dots inside the URL body must survive — only trailing punctuation is cut. + assert _url_netlocs("https://a.example.com/x.html, next") == {"a.example.com"} + assert metric_url_match("https://a.com/x", "https://b.com/x")[0] == 0.0 + + def test_rubric_json_parsers_tolerate_wrapping() -> None: assert _parse_result('result: {"result": 1, "reason": "ok"}') is True assert _parse_rubric_verdicts( From f42e5c7aa2917033940bd4dbb04054b5263e3b57 Mon Sep 17 00:00:00 2001 From: zhanghanduo Date: Wed, 26 Aug 2026 10:39:34 +0800 Subject: [PATCH 3/3] fix: fully redact summary LLM credentials --- frontier_agent/infra/summary_llm.py | 18 ++++-------------- tests/test_code_scanning_regressions.py | 13 +++++++------ 2 files changed, 11 insertions(+), 20 deletions(-) diff --git a/frontier_agent/infra/summary_llm.py b/frontier_agent/infra/summary_llm.py index 9b7c0aa..c43cbac 100644 --- a/frontier_agent/infra/summary_llm.py +++ b/frontier_agent/infra/summary_llm.py @@ -2,10 +2,7 @@ from __future__ import annotations import asyncio -import hashlib -import hmac import logging -import secrets from contextvars import ContextVar, Token from typing import Any @@ -24,7 +21,6 @@ _MAX_RETRIES = 4 _TRUNCATE_STEP = 40_960 _REQUEST_TIMEOUT = 300 -_KEY_FINGERPRINT_SECRET = secrets.token_bytes(32) # ── Profile-driven override ─────────────────────────────────────────── @@ -184,30 +180,24 @@ def _push( def describe_candidates(candidates: list[dict[str, Any]]) -> str: - """Printable candidate list with credentials reduced to a fingerprint. + """Printable candidate list with credentials fully redacted. :func:`summary_llm_candidates` returns live API keys, so printing its result verbatim writes them into terminal scrollback, CI logs and pasted bug reports. Debug through this instead: it keeps what identifies a candidate - (provider, model, endpoint) and reduces the key to its length plus a keyed, - process-local 12-hex-digit HMAC prefix — enough to tell two keys apart in - one run without enabling offline guesses from retained logs. + (provider, model, endpoint) and reports only whether a key is configured. + No value derived from the credential is retained or logged. """ if not candidates: return "(no summary LLM candidates)" lines: list[str] = [] for index, cand in enumerate(candidates, 1): key = str(cand.get("api_key") or "") - fingerprint = ( - f"len={len(key)} #" - f"{hmac.digest(_KEY_FINGERPRINT_SECRET, key.encode(), hashlib.sha256).hex()[:12]}" - if key else "unset" - ) lines.append( f"{index}. provider={cand.get('provider') or '?'}" f" model={cand.get('model') or '?'}" f" endpoint={cand.get('endpoint') or '?'}" - f" api_key={fingerprint}" + f" api_key={'set' if key else 'unset'}" ) return "\n".join(lines) diff --git a/tests/test_code_scanning_regressions.py b/tests/test_code_scanning_regressions.py index 57edf47..f5a6f92 100644 --- a/tests/test_code_scanning_regressions.py +++ b/tests/test_code_scanning_regressions.py @@ -59,16 +59,17 @@ def test_render_check_accepts_a_bom_anywhere_in_the_leading_run() -> None: assert not _looks_like_html_document("\ufeffplain text") -def test_api_key_fingerprint_is_keyed_and_does_not_expose_the_key() -> None: +def test_api_key_status_does_not_derive_or_expose_a_fingerprint() -> None: candidate = { "provider": "test", "model": "model", "endpoint": "https://example.test/v1", "api_key": "password-like-low-entropy-value", } - first = describe_candidates([candidate]) - second = describe_candidates([candidate]) + output = describe_candidates([candidate]) - assert first == second - assert candidate["api_key"] not in first - assert "len=31 #" in first + assert candidate["api_key"] not in output + assert "api_key=set" in output + assert "len=" not in output + assert "#" not in output + assert "api_key=unset" in describe_candidates([{**candidate, "api_key": ""}])