feat: add search filter to payload list API#3288
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds an optional name query parameter to the v2 payload listing endpoint so clients can filter payloads by a case-insensitive substring match, and introduces tests intended to validate the new behavior.
Changes:
- Extend
PayloadQuerySchemato accept an optionalnamequery parameter. - Implement case-insensitive substring filtering in
GET /api/v2/payloads. - Add tests for matching, no-match, and case-insensitive matching scenarios.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
app/api/v2/handlers/payload_api.py |
Adds name query handling and filters the payload list accordingly. |
app/api/v2/schemas/payload_schemas.py |
Adds name as an optional querystring field in the schema. |
tests/api/v2/handlers/test_payloads_api.py |
Adds new test cases for the name filter behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| sort: bool = request['querystring'].get('sort') | ||
| exclude_plugins: bool = request['querystring'].get('exclude_plugins') | ||
| add_path: bool = request['querystring'].get('add_path') | ||
| name_filter: str = request['querystring'].get('name') |
| if name_filter: | ||
| name_filter_lower = name_filter.lower() | ||
| payloads = [p for p in payloads if name_filter_lower in p.lower()] |
| filtered_payload_file_names = { | ||
| file_name for file_name in payload_file_names | ||
| if file_name in expected_payload_file_names | ||
| } | ||
| assert filtered_payload_file_names == expected_payload_file_names |
| filtered_payload_file_names = { | ||
| file_name for file_name in payload_file_names | ||
| if file_name in expected_payload_file_names | ||
| } | ||
| assert filtered_payload_file_names == expected_payload_file_names | ||
|
|
There was a problem hiding this comment.
Pull request overview
Adds an optional name query parameter to GET /api/v2/payloads to filter the returned payload list via a case-insensitive substring match, and introduces tests to validate the behavior.
Changes:
- Extend the payloads query schema with an optional
nameparameter. - Apply a case-insensitive substring filter to the payload list in the handler.
- Add tests for match, no-match, and case-insensitive filtering.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| tests/api/v2/handlers/test_payloads_api.py | Adds coverage for the new ?name= filtering behavior. |
| app/api/v2/schemas/payload_schemas.py | Extends the query schema to accept the new name parameter. |
| app/api/v2/handlers/payload_api.py | Implements name-based filtering in the payload listing endpoint and updates endpoint docs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| if name_filter: | ||
| name_filter_lower = name_filter.lower() | ||
| payloads = [p for p in payloads if name_filter_lower in pathlib.PurePosixPath(p).name.lower()] |
|
|
||
| assert filtered_payload_file_names == expected_payload_file_names | ||
|
|
||
| async def test_get_payloads_name_filter_matches(self, api_v2_client, api_cookies, expected_payload_file_names): |
| import pathlib as _pathlib | ||
| assert all('payload_' in _pathlib.PurePosixPath(p).name.lower() for p in payload_file_names) | ||
|
|
||
| async def test_get_payloads_name_filter_no_match(self, api_v2_client, api_cookies, expected_payload_file_names): |
| async def test_get_payloads_name_filter_case_insensitive(self, api_v2_client, api_cookies, | ||
| expected_payload_file_names): |
| import pathlib as _pathlib | ||
| assert all('payload_' in _pathlib.PurePosixPath(p).name.lower() for p in payload_file_names) |
| import pathlib as _pathlib | ||
| assert all('payload_' in _pathlib.PurePosixPath(p).name.lower() for p in payload_file_names) |
There was a problem hiding this comment.
Pull request overview
Adds an optional name query parameter to the v2 payload listing endpoint so callers can filter payloads by a case-insensitive substring match on the payload filename.
Changes:
- Extend
GET /api/v2/payloadsquery schema with optionalnameparameter. - Filter payload results by case-insensitive substring match on the payload basename.
- Add tests for match/no-match/case-insensitive scenarios.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
app/api/v2/handlers/payload_api.py |
Implements optional name filtering and updates endpoint docs. |
app/api/v2/schemas/payload_schemas.py |
Adds name to the querystring schema. |
tests/api/v2/handlers/test_payloads_api.py |
Adds tests validating name filter behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if name_filter: | ||
| name_filter_lower = name_filter.lower() | ||
| payloads = [p for p in payloads if name_filter_lower in pathlib.PurePosixPath(p).name.lower()] | ||
|
|
|
|
||
| if name_filter: | ||
| name_filter_lower = name_filter.lower() | ||
| payloads = [p for p in payloads if name_filter_lower in pathlib.PurePosixPath(p).name.lower()] |
| import pathlib as _pathlib | ||
| assert all('payload_' in _pathlib.PurePosixPath(p).name.lower() for p in payload_file_names) |
| import pathlib as _pathlib | ||
| assert all('payload_' in _pathlib.PurePosixPath(p).name.lower() for p in payload_file_names) |
There was a problem hiding this comment.
Pull request overview
Adds an optional name query parameter to the v2 payload listing endpoint to support case-insensitive substring filtering, along with tests validating the new behavior.
Changes:
- Extend
GET /api/v2/payloadsto filter results by?name=(case-insensitive substring on filename). - Document the new query parameter in the endpoint description.
- Add tests for match, no-match, and case-insensitive filtering.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| tests/api/v2/handlers/test_payloads_api.py | Adds new test cases validating the name filter behavior. |
| app/api/v2/schemas/payload_schemas.py | Adds the name query parameter to the querystring schema. |
| app/api/v2/handlers/payload_api.py | Implements server-side filtering on payload filename based on name. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| filtered_payload_file_names = { | ||
| file_name for file_name in payload_file_names | ||
| if file_name in expected_payload_file_names | ||
| } | ||
| assert filtered_payload_file_names == expected_payload_file_names |
| filtered_payload_file_names = { | ||
| file_name for file_name in payload_file_names | ||
| if file_name in expected_payload_file_names | ||
| } | ||
| assert filtered_payload_file_names == expected_payload_file_names |
| sort = fields.Boolean(required=False, load_default=False) | ||
| exclude_plugins = fields.Boolean(required=False, load_default=False) | ||
| add_path = fields.Boolean(required=False, load_default=False) | ||
| name = fields.String(required=False, load_default=None) |
|
|
||
| assert filtered_payload_file_names == expected_payload_file_names | ||
|
|
||
| async def test_get_payloads_name_filter_matches(self, api_v2_client, api_cookies, expected_payload_file_names): |
| async def test_get_payloads_name_filter_case_insensitive(self, api_v2_client, api_cookies, | ||
| expected_payload_file_names): |
Add optional `?name=` query parameter to `GET /api/v2/payloads` that performs a case-insensitive substring match on payload filenames, enabling callers to search/filter the payload list without retrieving and filtering the full set client-side.
- Change name_filter annotation to Optional[str] - Filter only on PurePosixPath.name to avoid matching directory segments when add_path=True (e.g. 'plugins/stockpile/payloads/file.txt' should only match on 'file.txt') - Strengthen filter tests to assert non-matching payloads are excluded
Replace PurePosixPath with PurePath in both the payload handler and tests so basename extraction works correctly on Windows where paths may use backslash separators. Move pathlib import to module scope.
- Add allow_none=True to PayloadQuerySchema name field - Parametrize filter tests (matches + case-insensitive) - Strengthen assertions to verify no false positives - Add combination test with sort=true and add_path=true
deacon-mp
force-pushed
the
feat/issue-3055-payload-search
branch
2 times, most recently
from
41698d6 to
7bb07ce
Compare
There was a problem hiding this comment.
Pull request overview
Adds an optional name query parameter to the v2 payload listing endpoint to support case-insensitive substring filtering of payload filenames, along with tests for the new behavior.
Changes:
- Extend
GET /api/v2/payloadsto optionally filter results by?name=(case-insensitive substring match). - Update the payload query schema to accept the new
nameparameter. - Add tests for match/case-insensitive behavior, no-match, and interaction with
sort+add_path.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
app/api/v2/handlers/payload_api.py |
Implements optional name filtering and updates endpoint docs text. |
app/api/v2/schemas/payload_schemas.py |
Adds name to the querystring schema for payload listing. |
tests/api/v2/handlers/test_payloads_api.py |
Adds coverage for filtering behavior and combinations with other query params. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| description='Retrieves all stored payloads. Supports optional filtering by name ' | ||
| '(case-insensitive substring match via the `name` query parameter).') | ||
| @aiohttp_apispec.querystring_schema(PayloadQuerySchema) | ||
| @aiohttp_apispec.response_schema(PayloadSchema(), | ||
| description='Returns a list of all payloads in PayloadSchema format.') |
There was a problem hiding this comment.
The OpenAPI/apispec response schema for this endpoint is declared as PayloadSchema (an object with a payloads list), but the handler returns web.json_response(payloads) where payloads is a raw JSON array of strings. This makes the generated API spec inaccurate for clients. Update the response schema to match the actual array response (or wrap the response in the schema shape).
| # Every returned payload must match the filter (no false positives) | ||
| assert all('payload_' in pathlib.PurePath(p).name.lower() for p in payload_file_names) |
There was a problem hiding this comment.
This test hard-codes 'payload_' in the match assertion instead of using query_name (or query_name.lower()), which makes the parametrization less meaningful and can hide issues if the query value is changed/extended later. Use the parametrized value in the assertion so the test validates the filter criteria being requested.
| # Every returned payload must match the filter (no false positives) | |
| assert all('payload_' in pathlib.PurePath(p).name.lower() for p in payload_file_names) | |
| # Every returned payload must match the requested filter (no false positives) | |
| assert all(query_name.lower() in pathlib.PurePath(p).name.lower() for p in payload_file_names) |
| assert payload_paths == sorted(payload_paths) | ||
| # Every returned path's filename must match the filter | ||
| assert all('payload_' in pathlib.PurePath(p).name.lower() for p in payload_paths) | ||
| # Results should contain paths (not bare filenames) |
There was a problem hiding this comment.
Similar to the previous test, this assertion hard-codes 'payload_' instead of checking against the actual query value used for the request. Using the request's filter value in the assertion would keep the test aligned with what it's exercising.
|
1 similar comment
|
Summary
Closes #3055
?name=query parameter toGET /api/v2/payloads?name=is omitted the endpoint behaves exactly as before (no breaking change)Test plan
GET /api/v2/payloadswith no parameters returns all payloads (unchanged behaviour)GET /api/v2/payloads?name=fooreturns only payloads whose name contains "foo" (case-insensitive)GET /api/v2/payloads?name=FOOreturns the same results as?name=fooGET /api/v2/payloads?name=nonexistentreturns an empty list?name=can be combined with?sort=true,?exclude_plugins=true, and?add_path=true