Skip to content

fix(flowfiles): bound all entries in pulled tar archives - #443

Open
yotatameguchi-boop wants to merge 1 commit into
vxcontrol:mainfrom
yotatameguchi-boop:fix/bound-pulled-tar-entries
Open

yotatameguchi-boop wants to merge 1 commit into
vxcontrol:mainfrom
yotatameguchi-boop:fix/bound-pulled-tar-entries

Conversation

@yotatameguchi-boop

Copy link
Copy Markdown

Description of the Change

Problem

ExtractTar limits regular files to 1,000 and their combined size to 2 GiB, but directory entries and skipped entries do not count toward either limit. A container directory with many empty subdirectories can therefore create an unbounded number of host-side staging directories during a pull. Skipped links and rejected paths also leave the number of iterations unbounded.

Solution

Add MaxPullEntries = 2 * MaxPullFiles and check it immediately after each successful tar.Reader.Next, before skipping or extracting the entry. Keep the existing regular-file and size limits. The 2,001st returned entry is rejected before it is processed.

No linked issue.

Type of Change

  • Bug fix
  • Test update
  • Security hardening

Areas Affected

  • Core Services: backend container-file pulls (pkg/flowfiles)

Testing and Verification

Test Configuration

  • Base commit: 843dfeb4bfc809c09d1bc5022e567c602d8fc9dd
  • Go: 1.26.5
  • Host: macOS / arm64
  • Docker, database, LLM provider: not needed for these package tests

Test Steps

  1. cd backend && go test ./pkg/flowfiles -count=1
  2. go vet ./pkg/flowfiles
  3. Regression check: remove only the new entry-count guard and its local counter, leaving the constant and tests in place, then run go test ./pkg/flowfiles -run TestFiles_ExtractTar_BoundsAllEntries -count=1.

Test Results

Package tests and vet pass. The regression check fails in all three new subtests (directories, symlinks, rejected paths) because the previous behavior accepts an archive over the entry limit. Restoring the guard makes the package pass again.

The new tests accept exactly 2,000 entries, reject 2,001, and verify that the excess entry is not created. Existing traversal, symlink, regular-file-count and content tests continue to pass.

Security Considerations

This bounds a previously unbounded dimension of container TAR processing. Pulling files requires authentication, access to the flow, and containers.view or containers.admin; this is not demonstrated as an unauthenticated attack. No new dependencies or permission changes. The existing service path closes the TAR stream and removes the staging directory on extraction errors.

This is a returned-entry limit, not a general byte/time limit for TAR parsing or concurrent requests. End-to-end service behavior and host exhaustion were not tested.

Performance Impact

One counter increment and comparison per returned entry; stops processing once the limit is exceeded. No benchmark performed.

Documentation Updates

The limit is documented beside the existing pull limits and at the check. No API schema or environment variable changes.

Deployment Notes

No migrations. Archives with more than 2,000 returned entries now fail, even if they contain fewer than 1,000 regular files. The 1,000-file and 2 GiB limits still apply independently. The new limit intentionally also counts skipped links and rejected paths.

Checklist

  • Code formatted with gofmt
  • Regression tests added
  • Affected package tests and go vet pass
  • Security implications and changed acceptance limit described
  • No dependencies, database schema, or API schema changes
  • Full backend / integration / end-to-end suite (not run)

Additional Notes

Prepared with AI assistance; validation was run locally against the affected package.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant