Skip to content

fix: enforce token file size limit before read#166

Merged
rsampaio merged 1 commit intomainfrom
fix-token-size-limit
Apr 14, 2026
Merged

fix: enforce token file size limit before read#166
rsampaio merged 1 commit intomainfrom
fix-token-size-limit

Conversation

@rsampaio
Copy link
Copy Markdown
Collaborator

@rsampaio rsampaio commented Apr 14, 2026

Reject oversized token files before reading regular files into memory so a misconfigured --token-file cannot bypass the 1 MiB guard and trigger excessive allocation.

Description

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

Summary by CodeRabbit

  • Bug Fixes

    • Enhanced token file validation during enrollment to check file size upfront and reject files exceeding the maximum allowed size (1 MiB) with a clear error message.
  • Tests

    • Added test coverage for oversized token file validation.

Reject oversized token files before reading regular files into memory so a misconfigured --token-file cannot bypass the 1 MiB guard and trigger excessive allocation.

Signed-off-by: Rodrigo Sampaio Vaz <rvaz@nvidia.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 14, 2026

📝 Walkthrough

Walkthrough

The resolveToken function in the enroll workflow now validates token file size upfront before reading. Instead of reading the file directly, the code explicitly opens the file, checks its size via Stat, rejects oversized files immediately, and then reads the content using a limited reader. A corresponding test verifies the oversized file rejection behavior.

Changes

Cohort / File(s) Summary
Token File Size Validation
cmd/fleetint/enroll.go
Modified file handling to perform size validation before reading: explicit os.Open, Stat check against maxTokenSize, and conditional error return; reads token via io.LimitReader instead of direct os.ReadFile.
Test Coverage
cmd/fleetint/security_test.go
Added TestResolveToken_RejectsOversizedTokenFile to verify that oversized token files are rejected with an "exceeds maximum size" error.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A token file too large to bear,
Now checked before we read with care,
With Stat we peek, with hops so bright,
Size validation done just right! ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the main change: enforcing token file size limits before reading, which directly addresses the security fix described in the PR objectives.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix-token-size-limit

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
cmd/fleetint/security_test.go (1)

137-152: Consider adding an explicit > maxTokenSize test case.

This test covers the boundary (== maxTokenSize) well. Adding a maxTokenSize+1 case would make the oversized contract unambiguous and guard against future comparator changes.

Suggested extension
 func TestResolveToken_RejectsOversizedTokenFile(t *testing.T) {
 	const maxTokenSize = 1 << 20
@@
 	require.NoError(t, os.Truncate(tmpFile, maxTokenSize))
@@
 	assert.Contains(t, err.Error(), "exceeds maximum size")
+
+	tmpFile2 := filepath.Join(t.TempDir(), "token-plus-one")
+	file2, err := os.Create(tmpFile2)
+	require.NoError(t, err)
+	require.NoError(t, file2.Close())
+	require.NoError(t, os.Truncate(tmpFile2, maxTokenSize+1))
+
+	err = app.Run([]string{"fleetint", "enroll", "--endpoint", "https://example.com", "--token-file", tmpFile2})
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "exceeds maximum size")
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@cmd/fleetint/security_test.go` around lines 137 - 152, Add a second assertion
case that explicitly tests a file larger than the allowed size: in
TestResolveToken_RejectsOversizedTokenFile (and using the existing maxTokenSize,
tmpFile and app.Run setup) create/truncate the tmpFile to maxTokenSize+1 bytes
and call app.Run with the same arguments, then require an error and assert the
error message contains "exceeds maximum size" to ensure the > maxTokenSize case
fails as expected.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@cmd/fleetint/security_test.go`:
- Around line 137-152: Add a second assertion case that explicitly tests a file
larger than the allowed size: in TestResolveToken_RejectsOversizedTokenFile (and
using the existing maxTokenSize, tmpFile and app.Run setup) create/truncate the
tmpFile to maxTokenSize+1 bytes and call app.Run with the same arguments, then
require an error and assert the error message contains "exceeds maximum size" to
ensure the > maxTokenSize case fails as expected.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: a2750104-408a-4e4c-9a32-493ea7c74380

📥 Commits

Reviewing files that changed from the base of the PR and between b305abd and a0e59af.

📒 Files selected for processing (2)
  • cmd/fleetint/enroll.go
  • cmd/fleetint/security_test.go

@rsampaio rsampaio merged commit db1e6e5 into main Apr 14, 2026
9 checks passed
@rsampaio rsampaio deleted the fix-token-size-limit branch April 14, 2026 21:35
jingxiang-z pushed a commit that referenced this pull request Apr 14, 2026
Signed-off-by: Rodrigo Sampaio Vaz <rvaz@nvidia.com>
jingxiang-z pushed a commit that referenced this pull request Apr 14, 2026
Signed-off-by: Rodrigo Sampaio Vaz <rvaz@nvidia.com>
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.

2 participants