Block admin and SHOW keywords in CypherQueryFilter #485
Conversation
There was a problem hiding this comment.
Pull request overview
This PR addresses security vulnerabilities by blocking administrative and information disclosure operations in user-provided Cypher queries. It adds GRANT, DENY, REVOKE, and SHOW keywords to the blocklist to prevent privilege escalation and information leakage via the #cypher_raw parser function.
Changes:
- Added four new keywords (GRANT, DENY, REVOKE, SHOW) to the CypherQueryFilter blocklist
- Added comprehensive test coverage including parameterized tests, realistic query examples, and false-positive string literal tests
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/CypherQueryFilter.php | Added GRANT, DENY, REVOKE, and SHOW to the WRITE_KEYWORDS constant array to block admin operations and information disclosure queries |
| tests/phpunit/CypherQueryFilterTest.php | Added new keywords to parameterized data provider and comprehensive write query test cases including admin operations (GRANT role, DENY read, REVOKE role) and SHOW commands (SHOW DATABASES, SHOW USERS), with false-positive tests for string literals |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| yield 'Lowercase grant' => [ | ||
| "grant role admin to user1", | ||
| 'Should reject lowercase grant' | ||
| ]; |
There was a problem hiding this comment.
Consider adding mixed case test coverage for the new admin keywords (DENY, REVOKE) and SHOW, similar to the existing 'Mixed case CrEaTe' test on line 207. This would ensure consistent test coverage across all blocked keywords.
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
|
@JeroenDeDauw I've opened a new pull request, #486, to work on those changes. Once the pull request is ready, I'll request review from you. |
* Initial plan * Add mixed case test coverage for DENY, REVOKE, and SHOW keywords Co-authored-by: JeroenDeDauw <146040+JeroenDeDauw@users.noreply.github.com> * Add mixed case test coverage for GRANT keyword for completeness Co-authored-by: JeroenDeDauw <146040+JeroenDeDauw@users.noreply.github.com> * Add lowercase test coverage for DENY and REVOKE keywords for consistency Co-authored-by: JeroenDeDauw <146040+JeroenDeDauw@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: JeroenDeDauw <146040+JeroenDeDauw@users.noreply.github.com>
Fixes #479
Fixes #480
Add SHOW, GRANT, DENY, and REVOKE to the CypherQueryFilter blocklist
to prevent information disclosure and admin operations via userland
Cypher queries.
For a more robust long-term approach, see
#484
Test plan
🤖 Generated with Claude Code