Flow
Order of checks on the apply request
1Login
Browser session or Basic header. Failure is a JSON 401.
2Folder list
Requested folder must be on the list or under it. Failure is a 403.
3Path test
Real path, or the parent for new files, must be inside the folder with the separator.
4Command list
Must be an array of exact matches. One miss is a 403 and the scratch folder is wiped.
5Checks, then copy
Only after all of that do files move into the project.
An apply endpoint in my coding assistant would run any shell command you put in the request. It never asked who you were. This is all from reading the code. I never saw an attack.
The setup is simple: a small web app sits in front of a local model, you take a snapshot of a project folder, the model proposes a bundle of changes, and you post that bundle to the apply endpoint. The bundle holds file edits, new files, a session id, and a list of commands to run after. The app stages the files in a scratch folder, runs checks, and copies them into the project if the checks pass.
Four ways in
There were four holes, and they stacked:
- No login on the API. Only the browser pages had one.
- The project folder came from the request. Any folder the server could read was fair game.
- The path check was a plain prefix test, so
/srv/shoplet in/srv/shop-old. It also turned away every new file. - The command list went straight to the shell.
The last one is the big one. Why break out of a folder when you can just ask for a shell?
Who, where, and what runs
Now every API call needs a login, either a browser session or a Basic auth header carrying the right password. A failure gets a JSON 401, not a redirect, since a script can't use a login page.
Allowed folders live in a small configuration file, and if that file is missing or broken, the app falls back to its own folder. A request for any other folder gets a 403.
File paths get one shared test. Existing files are checked by their real path. New files are checked through their parent folder. The test needs the separator, not just the prefix.
A path counts as inside only if it is the root or starts with the root followed by a separator, so a similarly named sibling folder doesn't pass.
Commands come from a second file. Each one has to match exactly. One unknown command kills the whole request, and the scratch folder is wiped. No file means no commands.
I like that the two files give way in opposite directions. No folder list still leaves one safe folder. No command list leaves nothing to run. Keep the approved commands as short named scripts, so you can read each one once.
Where it still bothers me
The session id comes from the request and goes onto the scratch folder path with no check. A rejected request deletes that folder. I think a nasty id could aim that delete somewhere else.
The approved commands don't run inside the scratch folder. So a test command isn't testing the new files.
The allowlist files sit inside the default allowed folder. A logged-in bundle could probably rewrite them.
And there's one shared password. Whoever has it can do everything the lists allow.