Skip to content

[php-wasm] Add support for rm command in sandboxed spawn handler #3010

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

Open
Utsav-Ladani wants to merge 2 commits into
base: trunk
Choose a base branch
from
+51 −3
Open

[php-wasm] Add support for rm command in sandboxed spawn handler #3010

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bc235d7
[php-wasm] Add support for `rm` command in sandboxed spawn handler
Utsav-Ladani Dec 10, 2025
89d198e
Add support for `-r` and `-f` command flags
Utsav-Ladani Dec 12, 2025
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
54 changes: 51 additions & 3 deletions packages/php-wasm/universal/src/lib/sandboxed-spawn-handler-factory.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@ import type { PHPWorker } from './php-worker';
import type { Remote } from './comlink-sync';
import { logger } from '@php-wasm/logger';

function wait(ms: number): Promise<void> {
return new Promise((resolve) => setTimeout(resolve, ms));
}

/**
* An isomorphic proc_open() handler that implements typical shell in TypeScript
* without relying on a server runtime. It can be used in the browser and Node.js
Expand Down Expand Up @@ -81,7 +85,7 @@ export function sandboxedSpawnHandlerFactory(
return;
}

if (!['php', 'ls', 'pwd'].includes(binaryName ?? '')) {
if (!['php', 'ls', 'pwd', 'rm'].includes(binaryName ?? '')) {
// 127 is the exit code "for command not found".
processApi.exit(127);
return;
Expand Down Expand Up @@ -142,7 +146,7 @@ export function sandboxedSpawnHandlerFactory(
// Technical limitation of subprocesses – we need to
// wait before exiting to give consumer a chance to read
// the output.
await new Promise((resolve) => setTimeout(resolve, 10));
await wait(10);
processApi.exit(0);
break;
}
Expand All @@ -151,10 +155,54 @@ export function sandboxedSpawnHandlerFactory(
// Technical limitation of subprocesses – we need to
// wait before exiting to give consumer a chance to read
// the output.
await new Promise((resolve) => setTimeout(resolve, 10));
await wait(10);
processApi.exit(0);
break;
}
case 'rm': {
const target = args[args.length - 1];
const flags = args.slice(1, -1).join(' ');
Copy link
Collaborator

@adamziel adamziel Dec 15, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

It may be more involved than that. Here's a few examples that would be really useful to cover with a unit test to make sure this command can handle them without failing:

rm file1 file2 file3
rm -r -f directory
rm file1 -f file2 -r
# This one means "remove a file called "-rf"
rm -- -rf
# This one means "recursively remove a file or a directory called "-rf"
rm -rf -- -rf

Copy link
Contributor Author

@Utsav-Ladani Utsav-Ladani Dec 16, 2025

Choose a reason for hiding this comment

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

@adamziel @zaerl What do you think about using the yargs-parser npm package to parse the args instead of writing a custom parser from scratch?

Copy link
Collaborator

@adamziel adamziel Dec 16, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

@Utsav-Ladani potentially! Let's give it a try and see what would this code look like with yargs-parser in place. It seems to be larger than we need, but still fairly small so perhaps? Note this package (@php-wasm/universal) can be imported in both node.js and web browser code, which yargs-parser seems to support. Also, one potentially relevant difference is that yargs-parser seems to accept a string, and in here we already have access to a pre-parsed args array where string syntax were already processed (e.g. "my string" becomes my string) – hopefully there's a way to work from there without re-encoding that and re-escaping quotes.


if (args.length < 2 || target.startsWith('-')) {
processApi.stderr('usage: rm [-rf] file\n');
// Technical limitation of subprocesses – we need to
// wait before exiting to give consumer a chance to read
// the output.
await wait(10);
processApi.exit(1);
break;
}

const isRecursive = flags.includes('r');
const isForce = flags.includes('f');

let errorMessage = '';

if (await php.isDir(target)) {
if (isRecursive) {
await php.rmdir(target, { recursive: true });
} else {
errorMessage = `rm: cannot remove '${target}': Is a directory\n`;
}
} else if (await php.isFile(target)) {
await php.unlink(target);
}
// Target doesn't exist and -f flag is not set
else if (!isForce) {
errorMessage = `rm: cannot remove '${target}': No such file or directory\n`;
}

if (errorMessage) {
processApi.stderr(errorMessage);
// Technical limitation of subprocesses – we need to
// wait before exiting to give consumer a chance to read
// the output.
await wait(10);
}

processApi.exit(errorMessage ? 1 : 0);
break;
}
}
} catch (e) {
// An exception here means the PHP runtime has crashed.
Expand Down
Loading