Skip to content

fix(api): harden PHP wrapper args for newer PHP versions #1901

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
elibosley merged 1 commit into from
Mar 11, 2026
Merged

fix(api): harden PHP wrapper args for newer PHP versions #1901

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 10 additions & 4 deletions api/src/core/utils/plugins/wrapper.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,17 @@
<?php
// Borrowed with love from https://b3z13r.wordpress.com/2011/05/16/passing-values-from-the-commandline-to-php-by-getpost-method/
// e.g. `./wrapper.php GET /tmp/random_file.php?arg1=true&arg2=a-really-long-string` { "username": "root" }
$method = $argv[1];
$query_parts = explode('?', $argv[2], 2);
$method = $argv[1] ?? 'GET';
$target = $argv[2] ?? '';
$query_parts = explode('?', $target, 2);
$file = $query_parts[0];
$query_params = $query_parts[1];
$body = $argv[3];
$query_params = $query_parts[1] ?? '';
$body = $argv[3] ?? '';

if ($file === '') {
fwrite(STDERR, "Missing PHP file path\n");
exit(1);
}
Comment on lines +11 to +14

@coderabbitai coderabbitai Bot Mar 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Also reject nonexistent or unreadable targets here.

This only catches an empty path. A typoed or unreadable path still falls through to include($file), so callers miss the wrapper’s explicit failure path and get PHP’s native warning behavior instead.

🛡️ Proposed hardening 
 if ($file === '') {
 	fwrite(STDERR, "Missing PHP file path\n");
 	exit(1);
 }
+
+if (!is_file($file) || !is_readable($file)) {
+	fwrite(STDERR, "PHP file not found or unreadable\n");
+	exit(1);
+}
In PHP CLI, what happens when include() is called on a nonexistent file, and how does that differ from require()? Please include whether execution continues and what exit status the process returns.
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@api/src/core/utils/plugins/wrapper.php` around lines 11 - 14, Update the
existing empty-path guard in wrapper.php to also validate that the target file
actually exists and is readable before calling include($file): after the ($file
=== '') check, add checks using file_exists($file) and is_readable($file) (or a
single is_readable() which implies existence) and, if they fail, fwrite(STDERR,
"Missing or unreadable PHP file path\n"); exit(1); so callers hit the wrapper’s
explicit failure path instead of PHP’s native include warning; keep the error
message and exit(1) behavior consistent with the current block that handles
empty paths.


// Load query_params or body into correct var
if ($method === 'GET') {
Expand Down
Loading