Skip to content

fix(tools): cap total size and file count when reading directories from the sandbox - #469

Open
Dextheking1 wants to merge 1 commit into
vxcontrol:mainfrom
Dextheking1:fix/terminal-read-total-cap
Open

Dextheking1 wants to merge 1 commit into
vxcontrol:mainfrom
Dextheking1:fix/terminal-read-total-cap

Conversation

@Dextheking1

Copy link
Copy Markdown

Problem

readFileFromContainer in backend/pkg/tools/terminal.go caps each single file at 100 MB, but when the path is a directory it appends every file to one in-memory buffer with no total size limit and no limit on the number of files. A directory holding a few hundred files just under the per-file cap grows the backend's memory until the process dies. EditFile reads through the same function, so an edit on such a directory hits it too.

The content of a flow's container is agent-controlled, and the backend is the process holding the Docker socket and the provider keys, so this takes down every flow, not just the one that caused it.

Solution

Count the entries and the total declared size while reading. Refuse the read past 1000 files, or past the same 100 MB in total, with errors in the existing style. Single file reads behave exactly as before, including the existing per-file error message.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Areas affected

  • Backend / API

Testing

Test cases

  1. New test TestTerminal_ReadFile_RefusesADirectoryThatOnlyExceedsTheLimitInTotal: a directory of three 40 MB files, each under the per-file limit, 120 MB together. Expected: the read is refused with the total size error.
  2. New test TestTerminal_ReadFile_RefusesAnArchiveWithTooManyFiles: a directory of 1001 small files. Expected: the read is refused with the entry count error.
  3. Existing ReadFile tests: whole content reads, the per-file limit refusal, and the malformed archive refusals all behave as before.

Verification steps

  1. go test ./pkg/tools/ -run TestTerminal_ReadFile: ok
  2. go vet ./pkg/tools/: clean
  3. go build ./pkg/tools/: ok

Security considerations

  • This PR does not introduce security vulnerabilities

Checklist

  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published

Additional notes

The limits reuse the existing 100 MB figure so a directory read and a single file read now share one budget. The files in a flow's work directory are listed and read as a set elsewhere too, so 1000 entries leaves plenty of room for normal work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

1 participant