diff --git a/github_app_mcp/README.md b/github_app_mcp/README.md index 576b18e..09825c6 100644 --- a/github_app_mcp/README.md +++ b/github_app_mcp/README.md @@ -137,9 +137,20 @@ access: | `list_merged_pull_requests` | Pull requests: read | With a supplied `ref`, `search_repository_content` scans an immutable snapshot: -at most 64 regular files, 256 KiB of eligible content (64 KiB per file), and +optional `path` selects an exact file or directory subtree at that commit. +Use a canonical repository-relative POSIX path without a trailing slash, +at most 240 characters and 16 components; globs and textual prefixes are not +supported. For example, `ref=, path="src/test"` searches only that +subtree, while `path="src/main.ts"` searches one file. Path resolution reads +nonrecursive trees from the pinned root, never the entire recursive root +inventory, and never follows symlinks or submodules. Missing paths fail +explicitly. `path` without `ref` is rejected, not translated into indexed search. + +Within the selected scope, at most 64 regular files, 256 KiB of eligible content +(64 KiB per file), and 66 content API requests for a full SHA (one lightweight `/git/commits` object, -one tree, and up to 64 blobs). Branch/tag inputs resolve through fixed +one tree, and up to 64 blobs) are allowed, plus at most one nonrecursive tree read +per path component. Branch/tag inputs resolve through fixed `/git/ref/heads`, `/git/ref/tags`, and at most eight `/git/tags` reads before the immutable commit-object read: at most ten additional requests, still inside the same scan deadline. `HEAD` first reads the repository's default branch. @@ -160,6 +171,27 @@ the scan. Anonymous fallback remains read-only and never authorizes writes. Without `ref`, search uses GitHub's indexed default-branch code search with its existing single-request 30-second HTTP timeout, not the scan budget. +Internal search-tree metadata has its own 1-MiB HTTP-response budget and is +never returned directly to the model. The generic 100,000-byte result guard +therefore does not reject a larger intermediate tree before matching. +Other HTTP responses remain capped at 128 KiB; model-facing match pages +and their MCP envelopes remain capped at 100,000 bytes. Truncated or oversized +trees and file/content/time budget exhaustion fail explicitly without partial +results or indexed fallback. Narrow `path` after an inventory or scan-limit +error, or use `get_file` at a full SHA if even a shallow tree exceeds its budget. +Do not just reduce `per_page`: that paginates matches, not the inventory scan. + +Pinned results return the selected `path` (`null` for the repository root), +`resolved_ref`, and repository-relative, commit-pinned matches sorted by path. +`total_count` counts matching files only within that scope. +`incomplete_results`, `skipped_files`, and `skipped_reasons` describe skipped +content within the chosen scope, not excluded paths elsewhere in the repository. +Even zero matches with `incomplete_results=false` establish coverage only for +that scope. Files over 64 KiB, binary/non-UTF-8 files, unsupported encodings, +symlinks, and submodules are explicitly skipped. Reuse the same `path`, +`resolved_ref`, and `per_page` for subsequent pages; restart at page 1 if changing +`per_page`. Reduce `per_page` only for an oversized match-page result. + ### Paged PR and commit reads Large changes use existing tools and ordinary Copilot tool/model turns, not diff --git a/github_app_mcp/src/issuelens_github_mcp/github.py b/github_app_mcp/src/issuelens_github_mcp/github.py index e354b0d..a777c66 100644 --- a/github_app_mcp/src/issuelens_github_mcp/github.py +++ b/github_app_mcp/src/issuelens_github_mcp/github.py @@ -35,6 +35,8 @@ _MAX_COMMIT_HTTP_RESPONSE_BYTES = 1024 * 1024 _MAX_FILE_BYTES = 64 * 1024 _MAX_QUERY_CHARS = 512 +_MAX_SEARCH_TREE_BYTES = 1024 * 1024 +_MAX_SEARCH_PATH_DEPTH = 16 _MAX_SEARCH_FILES = 64 _MAX_SEARCH_BYTES = 256 * 1024 _MAX_SEARCH_SECONDS = 60 @@ -448,7 +450,7 @@ async def list_repository_tree(self, repository: str, ref: str, *, recursive: bo async def search_repository_content( self, repository: str, query: str, *, ref: str | None = None, - per_page: int = 30, page: int = 1, + path: str | None = None, per_page: int = 30, page: int = 1, ) -> Any: repository = self._authorize(repository) if not isinstance(query, str) or any( @@ -459,6 +461,19 @@ async def search_repository_content( query = query.strip() if not query or len(query) > _MAX_QUERY_CHARS or _SEARCH_QUALIFIER.search(query): raise GitHubAppError("query must be plain text and within the bounded limit") + if path is not None: + if ref is None: + raise GitHubAppError("path requires ref for a pinned search", error_type="invalid_input") + path = _repository_path(path) + parts = pathlib.PurePosixPath(path).parts + if path != "/".join(parts) or any( + unicodedata.category(char).startswith("C") for char in path + ): + raise GitHubAppError("path must be a canonical repository-relative POSIX path", error_type="invalid_input") + if len(parts) > _MAX_SEARCH_PATH_DEPTH: + raise GitHubAppError( + f"path must have at most {_MAX_SEARCH_PATH_DEPTH} components", error_type="invalid_input", + ) pagination = _pagination(per_page, page) if ref is None: return await self._request( @@ -478,22 +493,23 @@ async def search_repository_content( ) stack.push_async_callback(client.aclose) result = await self._search_repository_content_at_ref( - repository, query, ref, per_page=per_page, page=page, client=client, + repository, query, ref, search_path=path, + per_page=per_page, page=page, client=client, ) if self._public_only: await self._require_public_repository(repository) deadline = scan_timeout.when() if deadline is not None and asyncio.get_running_loop().time() >= deadline: - raise GitHubAppError(budget_error) + raise GitHubAppError(budget_error, error_type="limit_exceeded") return result except TimeoutError: if not scan_timeout.expired(): raise - raise GitHubAppError(budget_error) from None + raise GitHubAppError(budget_error, error_type="limit_exceeded") from None async def _search_repository_content_at_ref( self, repository: str, query: str, ref: str, *, - per_page: int, page: int, client: httpx.AsyncClient, + search_path: str | None, per_page: int, page: int, client: httpx.AsyncClient, ) -> Any: if self._public_only: await self._require_public_repository(repository) @@ -516,36 +532,46 @@ async def _search_repository_content_at_ref( tree_sha = _search_oid(commit["tree"].get("sha")) if _SEARCH_OID_PATTERN.fullmatch(ref) and ref.lower() != resolved_ref: raise GitHubAppError("GitHub returned a different search commit") - tree = await self._request( - "GET", repository, f"/git/trees/{tree_sha}", - permissions={"contents": "read"}, params={"recursive": "1"}, - content_read_auth=content_read_auth, - _client=client, - ) - if ( - not isinstance(tree, Mapping) - or not isinstance(tree.get("tree"), list) - or type(tree.get("truncated")) is not bool - or _search_oid(tree.get("sha")) != tree_sha - ): - raise GitHubAppError("GitHub returned an invalid search tree") - if tree["truncated"]: - raise GitHubAppError("Repository search tree is truncated; scan cannot be complete") + prefix = "" + if search_path is not None: + parts = search_path.split("/") + for index, part in enumerate(parts): + entries = await self._read_search_tree( + repository, tree_sha, recursive=False, + content_read_auth=content_read_auth, client=client, + ) + selected = next((entry for entry in entries if entry["path"] == part), None) + if selected is None: + raise GitHubAppError( + "Repository search path was not found at the resolved commit", error_type="not_found", + ) + if selected["type"] == "tree": + tree_sha = _search_oid(selected["sha"]) + continue + if index != len(parts) - 1: + raise GitHubAppError( + "Repository search path cannot descend through a file, symlink, or submodule", + error_type="invalid_input", + ) + entries = [selected] + prefix = search_path.rpartition("/")[0] + break + else: + entries = await self._read_search_tree( + repository, tree_sha, recursive=True, + content_read_auth=content_read_auth, client=client, + ) + prefix = search_path + else: + entries = await self._read_search_tree( + repository, tree_sha, recursive=True, + content_read_auth=content_read_auth, client=client, + ) files = [] - seen_paths = set() skipped_reasons: dict[str, int] = {} - for entry in tree["tree"]: - if not isinstance(entry, Mapping): - raise GitHubAppError("GitHub returned an invalid search tree entry") - path = _repository_path(entry.get("path")) - if ( - path in seen_paths - or path != "/".join(pathlib.PurePosixPath(path).parts) - or any(unicodedata.category(char).startswith("C") for char in path) - ): - raise GitHubAppError("GitHub returned an invalid search tree path") - seen_paths.add(path) + for entry in entries: + path = _repository_path(f"{prefix}/{entry['path']}" if prefix else entry["path"]) blob_sha = _search_oid(entry.get("sha")) kind, mode = entry.get("type"), entry.get("mode") if kind == "tree" and mode == "040000": @@ -554,16 +580,17 @@ async def _search_repository_content_at_ref( reason = "symlink" if kind == "blob" else "submodule" skipped_reasons[reason] = skipped_reasons.get(reason, 0) + 1 continue - if kind != "blob" or mode not in ("100644", "100755"): - raise GitHubAppError("GitHub returned an unsupported search tree mode") - size = entry.get("size") - if type(size) is not int or size < 0: - raise GitHubAppError("GitHub returned an invalid search file size") - files.append((path, blob_sha, size)) + files.append((path, blob_sha, entry["size"])) if len(files) > _MAX_SEARCH_FILES: - raise GitHubAppError(f"Repository search exceeds {_MAX_SEARCH_FILES} regular files") + raise GitHubAppError( + f"Repository search exceeds {_MAX_SEARCH_FILES} regular files; narrow path, not per_page", + error_type="limit_exceeded", + ) if sum(size for _, _, size in files if size <= _MAX_FILE_BYTES) > _MAX_SEARCH_BYTES: - raise GitHubAppError(f"Repository search exceeds {_MAX_SEARCH_BYTES} content bytes") + raise GitHubAppError( + f"Repository search exceeds {_MAX_SEARCH_BYTES} content bytes; narrow path, not per_page", + error_type="limit_exceeded", + ) items = [] for path, blob_sha, size in sorted(files): @@ -591,12 +618,62 @@ async def _search_repository_content_at_ref( result = { "items": items[start:start + per_page], "total_count": len(items), "incomplete_results": bool(skipped_reasons), "resolved_ref": resolved_ref, + "path": search_path, "skipped_files": sum(skipped_reasons.values()), "skipped_reasons": skipped_reasons, } if len(json.dumps(result, ensure_ascii=True).encode("utf-8")) > _MAX_RESULT_BYTES: - raise GitHubAppError("GitHub search result is too large; reduce per_page") + raise GitHubAppError("GitHub search result is too large; reduce per_page", error_type="limit_exceeded") return result + async def _read_search_tree( + self, repository: str, tree_sha: str, *, recursive: bool, + content_read_auth: _ContentReadAuth, client: httpx.AsyncClient, + ) -> list[Mapping[str, Any]]: + tree = await self._request( + "GET", repository, f"/git/trees/{tree_sha}", + permissions={"contents": "read"}, params={"recursive": "1"} if recursive else None, + content_read_auth=content_read_auth, _client=client, _search_tree=True, + ) + if ( + not isinstance(tree, Mapping) + or not isinstance(tree.get("tree"), list) + or type(tree.get("truncated")) is not bool + or _search_oid(tree.get("sha")) != tree_sha + ): + raise GitHubAppError("GitHub returned an invalid search tree") + if tree["truncated"]: + raise GitHubAppError( + "Repository search tree is truncated; narrow path or use get_file with a full SHA", + error_type="limit_exceeded", + ) + entries = [] + seen_paths = set() + for entry in tree["tree"]: + if not isinstance(entry, Mapping): + raise GitHubAppError("GitHub returned an invalid search tree entry") + path = _repository_path(entry.get("path")) + if ( + path in seen_paths + or path != "/".join(pathlib.PurePosixPath(path).parts) + or any(unicodedata.category(char).startswith("C") for char in path) + or (not recursive and "/" in path) + ): + raise GitHubAppError("GitHub returned an invalid search tree path") + seen_paths.add(path) + _search_oid(entry.get("sha")) + kind, mode = entry.get("type"), entry.get("mode") + if (kind, mode) not in ( + ("tree", "040000"), ("blob", "120000"), ("commit", "160000"), + ("blob", "100644"), ("blob", "100755"), + ): + raise GitHubAppError("GitHub returned an unsupported search tree mode") + if kind == "blob" and mode in ("100644", "100755"): + size = entry.get("size") + if type(size) is not int or size < 0: + raise GitHubAppError("GitHub returned an invalid search file size") + entries.append(entry) + return entries + async def _resolve_search_commit( self, repository: str, ref: str, *, content_read_auth: _ContentReadAuth, client: httpx.AsyncClient, @@ -1221,6 +1298,7 @@ async def _request( _client: httpx.AsyncClient | None = None, _commit_detail: CommitDetail | None = None, _pull_request_file_manifest: bool = False, + _search_tree: bool = False, _api_version: str = _API_VERSION, _visibility_probe: bool = False, ) -> Any: @@ -1287,6 +1365,7 @@ async def _request( response.raise_for_status() content = bytearray() limit = ( + _MAX_SEARCH_TREE_BYTES if _search_tree else _MAX_COMMIT_HTTP_RESPONSE_BYTES if _commit_detail is not None or _pull_request_file_manifest else _MAX_HTTP_RESPONSE_BYTES @@ -1295,7 +1374,8 @@ async def _request( content.extend(chunk) if len(content) > limit: raise GitHubAppError( - "GitHub response is too large; narrow the request", + "Repository search tree metadata is too large; narrow path or use get_file with a full SHA" + if _search_tree else "GitHub response is too large; narrow the request", error_type="limit_exceeded", outcome="unknown" if write else "not_applied", ) @@ -1370,7 +1450,8 @@ async def _request( } for item in payload ] - if len(json.dumps(payload, ensure_ascii=True).encode("utf-8")) > _MAX_RESULT_BYTES: + # Search trees are internal metadata; only the constructed match page reaches the model. + if not _search_tree and len(json.dumps(payload, ensure_ascii=True).encode("utf-8")) > _MAX_RESULT_BYTES: raise GitHubAppError( "GitHub response is too large; narrow the request", error_type="limit_exceeded", outcome="unknown" if write else "not_applied", diff --git a/github_app_mcp/src/issuelens_github_mcp/server.py b/github_app_mcp/src/issuelens_github_mcp/server.py index e9530f9..bc6e41b 100644 --- a/github_app_mcp/src/issuelens_github_mcp/server.py +++ b/github_app_mcp/src/issuelens_github_mcp/server.py @@ -311,15 +311,27 @@ async def list_repository_tree(repository: str, ref: str, recursive: bool = True return await github.list_repository_tree(repository, ref, recursive=recursive) @server.tool() - async def search_repository_content(repository: str, query: str, ref: str | None = None, per_page: int = 30, page: int = 1) -> Any: + async def search_repository_content( + repository: str, query: str, ref: str | None = None, + per_page: _PerPage = 30, page: Annotated[int, Field(strict=True, ge=1, le=100)] = 1, + path: str | None = None, + ) -> Any: """Search content in one explicit repository; query cannot contain qualifiers. With ref, resolve a branch, tag, or commit to one immutable commit and search its regular UTF-8 files for the trimmed, case-insensitive literal query within each line, not in paths. No regex or search operators apply. - At most 64 regular files, 256 KiB eligible content, and 66 content API - requests are allowed for a full SHA, plus initial authentication - lookup/minting overhead. Branch/tag resolution uses fixed Git-ref/tag + Optional path selects one exact file or directory subtree, not a glob + or textual prefix. It requires ref and a canonical repository-relative + POSIX path (no trailing slash), at most 240 characters/16 components. + Resolve it through nonrecursive trees at the pinned commit, then scan + only that subtree; symlinks/submodules are never followed. Omitting path + scans the root. The same 64-regular-file/256-KiB eligible-content caps + apply within the chosen scope. Narrow path after a scan-limit error; + reducing per_page only shrinks returned matches, not the scan. + A full SHA uses at most 66 content API requests plus one nonrecursive + tree read per path component, plus initial authentication lookup/minting + overhead. Branch/tag resolution uses fixed Git-ref/tag routes (up to ten additional requests), never patch-bearing commits. One HTTP client is reused for the scan's content requests and closed on success, error, cancellation, or deadline expiry. A fixed 60-second overall @@ -329,20 +341,26 @@ async def search_repository_content(repository: str, query: str, ref: str | None propagates after cleanup. Files over 64 KiB, binary/non-UTF-8 content, unsupported encodings, symlinks, and submodules are skipped explicitly. Truncated trees, exhausted scan limits, and malformed responses fail without an - indexed fallback. HTTP responses are capped at 128 KiB and returned - results at 100,000 bytes. + indexed fallback. Internal tree downloads have a separate 1-MiB budget + per response; other HTTP responses remain capped at 128 KiB. Only the + constructed match page is subject to the 100,000-byte model-result cap. Items are sorted by path then paginated; total_count counts matching - files in the scanned subset. Check incomplete_results and skipped_reasons - before treating zero matches as exhaustive. Each item has a blob SHA, - commit-pinned URL, and at most three matching line numbers with excerpts - capped at the first 160 characters. Reuse resolved_ref for later pages - to avoid a moving branch changing the snapshot; reduce per_page if the - result exceeds the response limit. + files within returned path (null means repository root), not the whole + repository when scoped. incomplete_results/skipped_reasons describe only + that scope; check them before treating zero matches as exhaustive there. + Missing paths fail explicitly, not as empty successful scans. + Each item has a blob SHA, commit-pinned URL, and at most three matching + line numbers with excerpts capped at the first 160 characters. + Reuse resolved_ref and the same path for later pages to keep scope and snapshot; + restart at page 1 after changing per_page. Reduce per_page if the result + exceeds the response limit. Without ref, use GitHub's indexed default-branch code search and return its native result/pagination semantics, not immutable source evidence. """ - return await github.search_repository_content(repository, query, ref=ref, per_page=per_page, page=page) + return await github.search_repository_content( + repository, query, ref=ref, path=path, per_page=per_page, page=page, + ) @server.tool() async def list_merged_pull_requests(repository: str, base: str, since: str | None = None, per_page: int = 30, page: int = 1) -> Any: diff --git a/github_app_mcp/tests/test_public_access.py b/github_app_mcp/tests/test_public_access.py index 3a7916c..e8c2603 100644 --- a/github_app_mcp/tests/test_public_access.py +++ b/github_app_mcp/tests/test_public_access.py @@ -82,6 +82,7 @@ async def test_nonpublic_or_unverified_repositories_deny_every_data_path_before_ "tree": lambda c: c.list_repository_tree(REPOSITORY, COMMIT), "indexed source": lambda c: c.search_repository_content(REPOSITORY, "crash"), "pinned source": lambda c: c.search_repository_content(REPOSITORY, "crash", ref=COMMIT), + "scoped pinned source": lambda c: c.search_repository_content(REPOSITORY, "crash", ref=COMMIT, path="src"), "merged PRs": lambda c: c.list_merged_pull_requests(REPOSITORY, base="main"), "wiki": lambda c: c.get_wiki_snapshot(REPOSITORY), "wiki pages": lambda c: c.list_wiki_pages(REPOSITORY), @@ -281,6 +282,52 @@ def material(request): self.assertNotIn(CANARY, json.dumps(result)) self.assertTrue(any(r.url.host == "private-user-images.githubusercontent.com" for r in self.requests)) + async def test_scoped_search_rechecks_visibility_before_releasing_tree_or_match_data(self): + subtree = "d" * 40 + for changed_after in ("ancestor", "subtree", "blob"): + with self.subTest(changed_after=changed_after): + self.requests.clear() + nonpublic = False + + def metadata(): + return {**PUBLIC, "private": True, "visibility": "private"} if nonpublic else PUBLIC + + def material(request): + nonlocal nonpublic + if request.url.path.endswith(f"/git/commits/{COMMIT}"): + return httpx.Response(200, json={"sha": COMMIT, "tree": {"sha": TREE}}) + if request.url.path.endswith(f"/git/trees/{TREE}"): + self.assertFalse(request.url.params) + nonpublic = changed_after == "ancestor" + return httpx.Response(200, json={ + "sha": TREE, "truncated": False, + "tree": [{"path": "src", "sha": subtree, "type": "tree", "mode": "040000"}], + }) + if request.url.path.endswith(f"/git/trees/{subtree}"): + self.assertEqual(request.url.params["recursive"], "1") + nonpublic = changed_after == "subtree" + return httpx.Response(200, json={ + "sha": subtree, "truncated": False, + "tree": [{"path": "source.py", "sha": BLOB, "type": "blob", "mode": "100644", "size": len(CANARY)}], + }) + self.assertTrue(request.url.path.endswith(f"/git/blobs/{BLOB}")) + nonpublic = changed_after == "blob" + return httpx.Response(200, json={ + "sha": BLOB, "size": len(CANARY), "encoding": "base64", + "content": base64.b64encode(CANARY.encode()).decode(), + }) + + client = self.client(metadata, material=material) + result = await create_server(client).call_tool("search_repository_content", { + "repository": REPOSITORY, "query": CANARY, "ref": COMMIT, "path": "src", + }) + self.assertTrue(result.is_error) + self.assertEqual(result.structured_content["error"]["type"], "permission_denied") + self.assertIsNone(result.structured_content["result"]) + self.assertNotIn(CANARY, str(result)) + content_reads = [r for r in self.requests if r.url.path != f"/repos/{REPOSITORY}"] + self.assertEqual(len(content_reads), {"ancestor": 2, "subtree": 3, "blob": 4}[changed_after]) + async def test_public_wiki_rejects_nonpublic_destination_before_git_access(self): client = self.client(PUBLIC) original = client.get_repository diff --git a/github_app_mcp/tests/test_server.py b/github_app_mcp/tests/test_server.py index 53dc93d..409ff6d 100644 --- a/github_app_mcp/tests/test_server.py +++ b/github_app_mcp/tests/test_server.py @@ -324,6 +324,32 @@ async def test_write_tools_are_registered_only_when_enabled(self): READ_TOOLS | WRITE_TOOLS, ) + async def test_pinned_search_scope_and_pagination_are_discoverable_and_forwarded(self): + github = FakeGitHubClient() + async with Client(create_server(cast(GitHubClient, github))) as client: + tools = await client.list_tools() + result = await client.call_tool("search_repository_content", { + "repository": "microsoft/IssueLens", "query": "needle", + "ref": "a" * 40, "path": "src/test", "per_page": 2, "page": 3, + }) + for invalid in ({"per_page": True}, {"page": 101}, {"path": 123}): + with self.subTest(invalid=invalid): + rejected = await client.call_tool("search_repository_content", { + "repository": "microsoft/IssueLens", "query": "needle", **invalid, + }) + self.assertTrue(rejected.is_error) + self.assertEqual(rejected.structured_content["error"]["type"], "invalid_input") + search_tool = next(tool for tool in tools.tools if tool.name == "search_repository_content") + properties = search_tool.input_schema["properties"] + self.assertIsNone(properties["path"]["default"]) + self.assertEqual(properties["per_page"]["maximum"], 100) + self.assertEqual(properties["page"]["maximum"], 100) + self.assertIn("reducing per_page only shrinks returned matches", search_tool.description) + self.assertFalse(result.is_error) + self.assertEqual(github.calls, [("search_repository_content", ("microsoft/IssueLens", "needle"), { + "ref": "a" * 40, "path": "src/test", "per_page": 2, "page": 3, + })]) + async def test_wiki_writer_has_only_direct_mutation_and_bounded_schema(self): github = FakeGitHubClient(wiki_writes_enabled=True) server = create_server(cast(GitHubClient, github)) diff --git a/skills/change-analysis/SKILL.md b/skills/change-analysis/SKILL.md index 70833d6..64115de 100644 --- a/skills/change-analysis/SKILL.md +++ b/skills/change-analysis/SKILL.md @@ -90,8 +90,11 @@ instead of claiming full coverage. exact or complete PR patch. - `get_file` supports bounded UTF-8 content up to 64 KiB, not arbitrary file ranges or automatic large-resource downloads. A full-SHA - `search_repository_content` can answer a targeted question, but its bounded - matches and `incomplete_results` do not prove whole-file or whole-PR coverage. + `search_repository_content` can answer a targeted question. Use its optional + `path` to select an exact file or directory subtree after an inventory/scan + limit; `per_page` only paginates matches, not the scan. Preserve `path` and + `resolved_ref` across pages. Its bounded matches and `incomplete_results` + describe only the chosen scope, not whole-file or whole-PR coverage. - PR manifests and PR/commit file lists have a 3,000-file ceiling. REST file tools permit at most 3,000 pages so single-file paging remains possible; other numbered-page reads remain capped at 100 pages. The PR-commit endpoint diff --git a/tests/test_repository_search.py b/tests/test_repository_search.py index 0ca6bfc..32f7ea3 100644 --- a/tests/test_repository_search.py +++ b/tests/test_repository_search.py @@ -113,6 +113,14 @@ def add_file(self, path, content, *, tree=OLD_TREE, mode="100644"): } return blob_sha + def add_directory(self, path, *, tree=OLD_TREE): + tree_sha = hashlib.sha1(f"{tree}/{path}".encode("utf-8")).hexdigest() + self.trees[tree]["tree"].append({ + "path": path, "type": "tree", "mode": "040000", "sha": tree_sha, + }) + self.trees[tree_sha] = {"sha": tree_sha, "truncated": False, "tree": []} + return tree_sha + def handler(self, request): self.requests.append(request) self.assertEqual(request.method, "GET") @@ -143,7 +151,7 @@ def handler(self, request): commit = self.commits.get(ref.lower()) return httpx.Response(200, json=commit) if commit is not None else httpx.Response(404) if route.startswith("git/trees/"): - self.assertEqual(request.url.params["recursive"], "1") + self.assertIn(dict(request.url.params), ({}, {"recursive": "1"})) return httpx.Response(200, json=self.trees[route.removeprefix("git/trees/")]) if route.startswith("git/blobs/"): return httpx.Response(200, json=self.blobs[route.removeprefix("git/blobs/")]) @@ -1101,6 +1109,7 @@ async def test_skipped_files_are_explicit_and_never_follow_links(self): async def test_empty_snapshot_is_complete(self): result = await self.client.search_repository_content(REPOSITORY, "needle", ref=OLD_COMMIT) self.assertEqual(result["items"], []) + self.assertIsNone(result["path"]) self.assertEqual(result["total_count"], 0) self.assertEqual(result["skipped_reasons"], {}) self.assertFalse(result["incomplete_results"]) @@ -1122,8 +1131,9 @@ async def test_aggregate_result_limit_and_smaller_page(self): content = (("needle" + "\u4e00" * 154 + "\n") * 3).encode("utf-8") for index in range(64): self.add_file(f"{index:02}.txt", content) - with self.assertRaisesRegex(GitHubAppError, "reduce per_page"): + with self.assertRaisesRegex(GitHubAppError, "reduce per_page") as caught: await self.client.search_repository_content(REPOSITORY, "needle", ref=OLD_COMMIT, per_page=100) + self.assertEqual(caught.exception.error_type, "limit_exceeded") result = await self.client.search_repository_content(REPOSITORY, "needle", ref=OLD_COMMIT, per_page=1) self.assertEqual(len(result["items"]), 1) self.assertEqual(result["total_count"], 64) @@ -1136,10 +1146,227 @@ async def test_line_numbers_count_source_newlines_not_unicode_separators(self): {"line_number": 2, "excerpt": "needle", "truncated": False}, ]) - async def test_oversized_tree_honors_request_result_limit(self): + async def test_internal_tree_metadata_is_not_a_model_result(self): + self.add_file("source.txt", b"needle") self.trees[OLD_TREE]["padding"] = "x" * 100_000 + response = httpx.Response(200, json=self.trees[OLD_TREE]) + self.assertLess(len(response.content), github_module._MAX_HTTP_RESPONSE_BYTES) + self.assertGreater( + len(json.dumps(self.trees[OLD_TREE]).encode("utf-8")), + github_module._MAX_RESULT_BYTES, + ) + result = await self.client.search_repository_content(REPOSITORY, "needle", ref=OLD_COMMIT) + self.assertEqual(result["total_count"], 1) + self.assertNotIn("padding", result) + self.assertLess(len(json.dumps(result).encode("utf-8")), github_module._MAX_RESULT_BYTES) with self.assertRaisesRegex(GitHubAppError, "too large"): + await self.client.list_repository_tree(REPOSITORY, OLD_TREE) + + async def test_path_avoids_large_root_inventory_and_unrelated_scan_budgets(self): + for index in range(338): + self.add_file(f"{index:03}.txt", b"x" * 5000) + subtree = self.add_directory("src") + blob = self.add_file("source.py", b"needle", tree=subtree) + self.add_directory("src-other") + self.trees[OLD_TREE]["padding"] = "x" * 100_000 + self.assertGreater( + len(httpx.Response(200, json=self.trees[OLD_TREE]).content), + github_module._MAX_HTTP_RESPONSE_BYTES, + ) + for per_page in (1, 100): + with self.subTest(per_page=per_page): + self.requests.clear() + with self.assertRaisesRegex(GitHubAppError, "64 regular files; narrow path") as caught: + await self.client.search_repository_content( + REPOSITORY, "needle", ref=OLD_COMMIT, per_page=per_page, + ) + self.assertEqual(caught.exception.error_type, "limit_exceeded") + self.assertEqual(caught.exception.outcome, "not_applied") + self.assertEqual(len(self.requests), 2) + + self.requests.clear() + result = await self.client.search_repository_content( + REPOSITORY, "needle", ref=OLD_COMMIT, path="src", + ) + self.assertEqual(result["path"], "src") + self.assertEqual(result["resolved_ref"], OLD_COMMIT) + self.assertEqual(result["total_count"], 1) + self.assertFalse(result["incomplete_results"]) + self.assertEqual(result["items"][0]["path"], "src/source.py") + self.assertEqual(result["items"][0]["sha"], blob) + self.assertEqual(result["items"][0]["html_url"], + f"https://github.com/{REPOSITORY}/blob/{OLD_COMMIT}/src/source.py") + self.assertEqual( + [(r.url.path, dict(r.url.params)) for r in self.requests], + [(f"/repos/{REPOSITORY}/git/commits/{OLD_COMMIT}", {}), + (f"/repos/{REPOSITORY}/git/trees/{OLD_TREE}", {}), + (f"/repos/{REPOSITORY}/git/trees/{subtree}", {"recursive": "1"}), + (f"/repos/{REPOSITORY}/git/blobs/{blob}", {})], + ) + + async def test_nested_directory_and_exact_file_scopes_preserve_paths_and_pagination(self): + subtree = self.add_directory("src") + nested = self.add_directory("space # \u00fc", tree=subtree) + self.add_file("z.py", b"needle last", tree=nested) + first_blob = self.add_file("a.py", b"needle first", tree=nested) + self.add_file("a.py-extra", b"needle unrelated", tree=subtree) + search_path = "src/space # \u00fc" + first = await self.client.search_repository_content( + REPOSITORY, "needle", ref="release/topic", path=search_path, per_page=1, + ) + self.refs["release/topic"] = NEW_COMMIT + second = await self.client.search_repository_content( + REPOSITORY, "needle", ref=first["resolved_ref"], path=search_path, per_page=1, page=2, + ) + past_end = await self.client.search_repository_content( + REPOSITORY, "needle", ref=first["resolved_ref"], path=search_path, per_page=1, page=3, + ) + self.assertEqual(first["total_count"], 2) + self.assertEqual(second["total_count"], 2) + self.assertEqual(first["path"], second["path"]) + self.assertEqual(second["resolved_ref"], OLD_COMMIT) + self.assertEqual(first["items"][0]["path"], f"{search_path}/a.py") + self.assertEqual(second["items"][0]["path"], f"{search_path}/z.py") + self.assertEqual(past_end["items"], []) + self.assertEqual(past_end["total_count"], 2) + self.assertIn("/src/space%20%23%20%C3%BC/a.py", first["items"][0]["html_url"]) + + self.requests.clear() + exact = await self.client.search_repository_content( + REPOSITORY, "needle", ref=OLD_COMMIT, path=f"{search_path}/a.py", + ) + self.assertEqual(exact["path"], f"{search_path}/a.py") + self.assertEqual(exact["items"], first["items"]) + self.assertEqual(exact["total_count"], 1) + self.assertEqual(exact["items"][0]["sha"], first_blob) + self.assertEqual(len(self.requests), 5) + self.assertTrue(all(not r.url.params for r in self.requests)) + + async def test_scope_keeps_missing_and_skipped_coverage_explicit(self): + subtree = self.add_directory("src") + self.add_file("outside.txt", b"needle") + self.add_file("binary.bin", b"needle\x00", tree=subtree) + self.add_file("large.txt", b"x" * (64 * 1024 + 1), tree=subtree) + self.add_file("link", b"../outside.txt", tree=subtree, mode="120000") + self.trees[subtree]["tree"].append({ + "path": "module", "type": "commit", "mode": "160000", "sha": NEW_COMMIT, + }) + result = await self.client.search_repository_content( + REPOSITORY, "needle", ref=OLD_COMMIT, path="src", + ) + self.assertEqual(result["items"], []) + self.assertEqual(result["total_count"], 0) + self.assertTrue(result["incomplete_results"]) + self.assertEqual(result["skipped_files"], 4) + self.assertEqual(result["skipped_reasons"], { + "binary": 1, "file_too_large": 1, "symlink": 1, "submodule": 1, + }) + for path, reason in (("src/link", "symlink"), ("src/module", "submodule")): + with self.subTest(path=path): + result = await self.client.search_repository_content( + REPOSITORY, "needle", ref=OLD_COMMIT, path=path, + ) + self.assertEqual(result["items"], []) + self.assertEqual(result["path"], path) + self.assertTrue(result["incomplete_results"]) + self.assertEqual(result["skipped_reasons"], {reason: 1}) + self.add_directory("empty") + empty = await self.client.search_repository_content( + REPOSITORY, "needle", ref=OLD_COMMIT, path="empty", + ) + self.assertFalse(empty["incomplete_results"]) + self.assertEqual(empty["path"], "empty") + self.assertEqual(empty["total_count"], 0) + for path, error_type in ( + ("missing", "not_found"), ("Src", "not_found"), ("src/missing", "not_found"), + ("src/link/child", "invalid_input"), ("src/module/child", "invalid_input"), + ("outside.txt/child", "invalid_input"), + ): + with self.subTest(path=path): + with self.assertRaises(GitHubAppError) as caught: + await self.client.search_repository_content(REPOSITORY, "needle", ref=OLD_COMMIT, path=path) + self.assertEqual(caught.exception.error_type, error_type) + self.assertFalse(any(r.url.path == "/search/code" for r in self.requests)) + + async def test_invalid_paths_fail_before_authentication(self): + for path in ( + "", ".", "..", "/src", "src/", "src//test", "src/./test", "src/../test", + "src\\test", "src\n", "src\u200b", 123, [], {}, "x" * 241, + "/".join(["src"] * 17), + ): + with self.subTest(path=path): + with self.assertRaises(GitHubAppError) as caught: + await self.client.search_repository_content(REPOSITORY, "needle", ref=OLD_COMMIT, path=path) + self.assertEqual(caught.exception.error_type, "invalid_input") + with self.assertRaisesRegex(GitHubAppError, "path requires ref"): + await self.client.search_repository_content(REPOSITORY, "needle", path="src") + self.assertEqual(self.provider.calls, []) + self.assertEqual(self.requests, []) + + async def test_maximum_path_depth_is_bounded_and_accepted(self): + subtree = OLD_TREE + for _ in range(github_module._MAX_SEARCH_PATH_DEPTH): + subtree = self.add_directory("d", tree=subtree) + self.add_file("source.py", b"needle", tree=subtree) + search_path = "/".join(["d"] * github_module._MAX_SEARCH_PATH_DEPTH) + result = await self.client.search_repository_content( + REPOSITORY, "needle", ref=OLD_COMMIT, path=search_path, + ) + self.assertEqual(result["items"][0]["path"], f"{search_path}/source.py") + self.assertEqual(len(self.requests), 3 + github_module._MAX_SEARCH_PATH_DEPTH) + + async def test_scoped_search_still_enforces_file_and_content_budgets(self): + subtree = self.add_directory("src") + for count, size, message in ((65, 6, "64 regular files"), (5, 64 * 1024, "262144 content bytes")): + self.trees[subtree]["tree"] = [] + for index in range(count): + self.add_file(f"{index:02}.py", b"needle" + b"x" * (size - 6), tree=subtree) + for per_page in (1, 100): + with self.subTest(count=count, per_page=per_page): + self.requests.clear() + with self.assertRaisesRegex(GitHubAppError, message) as caught: + await self.client.search_repository_content( + REPOSITORY, "needle", ref=OLD_COMMIT, path="src", per_page=per_page, + ) + self.assertEqual(caught.exception.error_type, "limit_exceeded") + self.assertEqual(caught.exception.outcome, "not_applied") + self.assertEqual(len(self.requests), 3) + + async def test_path_resolution_rejects_truncated_or_malformed_ancestors(self): + subtree = self.add_directory("src") + self.add_file("source.py", b"needle", tree=subtree) + valid = copy.deepcopy(self.trees[OLD_TREE]) + entry = valid["tree"][0] + for tree in ( + {**valid, "truncated": True}, + {**valid, "sha": NEW_TREE}, + {**valid, "tree": [entry, entry]}, + {**valid, "tree": [{**entry, "path": "src/nested"}]}, + {**valid, "tree": [{**entry, "mode": []}]}, + ): + with self.subTest(tree=tree): + self.requests.clear() + self.trees[OLD_TREE] = tree + with self.assertRaises(GitHubAppError): + await self.client.search_repository_content(REPOSITORY, "needle", ref=OLD_COMMIT, path="src") + self.assertEqual(len(self.requests), 2) + + async def test_internal_tree_download_budget_remains_bounded(self): + self.add_file("source.py", b"needle") + self.trees[OLD_TREE]["padding"] = "" + encoded = json.dumps(self.trees[OLD_TREE]).encode("utf-8") + self.trees[OLD_TREE]["padding"] = "x" * (github_module._MAX_SEARCH_TREE_BYTES - len(encoded)) + encoded = json.dumps(self.trees[OLD_TREE]).encode("utf-8") + self.assertEqual(len(encoded), github_module._MAX_SEARCH_TREE_BYTES) + self.overrides[f"git/trees/{OLD_TREE}"] = httpx.Response(200, content=encoded) + result = await self.client.search_repository_content(REPOSITORY, "needle", ref=OLD_COMMIT) + self.assertEqual(result["total_count"], 1) + self.overrides[f"git/trees/{OLD_TREE}"] = httpx.Response(200, content=encoded + b" ") + self.requests.clear() + with self.assertRaisesRegex(GitHubAppError, "tree metadata is too large; narrow path") as caught: await self.client.search_repository_content(REPOSITORY, "needle", ref=OLD_COMMIT) + self.assertEqual(caught.exception.error_type, "limit_exceeded") + self.assertEqual(caught.exception.outcome, "not_applied") self.assertEqual(len(self.requests), 2) async def test_malformed_commit_payloads_fail_safely(self):