Skip to content

Commit bad8359

Browse files
sallyomclaude
andcommitted
upload-files feature: RBAC ordering and file type validation
fixing authentication pattern violation and adding MIME type validation via magic bytes to prevent Content-Type spoofing. - Fix RBAC check ordering in DeleteSessionWorkspaceFile to prevent session enumeration - Add file-type package for magic byte detection on all file uploads - Validate actual file content against claimed Content-Type headers for both local and URL uploads Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: sallyom <somalley@redhat.com>
1 parent 49106e0 commit bad8359

File tree

4 files changed

+227
-18
lines changed

4 files changed

+227
-18
lines changed

components/backend/handlers/sessions.go

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2754,18 +2754,6 @@ func DeleteSessionWorkspaceFile(c *gin.Context) {
27542754
return
27552755
}
27562756

2757-
// Verify session exists using reqDyn before RBAC check to prevent session enumeration
2758-
gvr := GetAgenticSessionV1Alpha1Resource()
2759-
if _, err := reqDyn.Resource(gvr).Namespace(project).Get(c.Request.Context(), session, v1.GetOptions{}); err != nil {
2760-
if errors.IsNotFound(err) {
2761-
c.JSON(http.StatusNotFound, gin.H{"error": "Session not found"})
2762-
return
2763-
}
2764-
log.Printf("DeleteSessionWorkspaceFile: Failed to verify session existence: %v", err)
2765-
c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to verify session"})
2766-
return
2767-
}
2768-
27692757
// Validate and sanitize path to prevent directory traversal
27702758
// Use robust path validation that works across platforms
27712759
sub := strings.TrimPrefix(c.Param("path"), "/")
@@ -2792,6 +2780,7 @@ func DeleteSessionWorkspaceFile(c *gin.Context) {
27922780
}
27932781

27942782
// RBAC check: verify user has update permission on agenticsessions (file operations modify session state)
2783+
// IMPORTANT: RBAC check MUST happen BEFORE checking session existence to prevent enumeration attacks
27952784
ssar := &authzv1.SelfSubjectAccessReview{
27962785
Spec: authzv1.SelfSubjectAccessReviewSpec{
27972786
ResourceAttributes: &authzv1.ResourceAttributes{
@@ -2813,6 +2802,19 @@ func DeleteSessionWorkspaceFile(c *gin.Context) {
28132802
return
28142803
}
28152804

2805+
// Verify session exists using reqDyn AFTER RBAC check
2806+
// This prevents enumeration attacks - unauthorized users get same "Forbidden" response
2807+
gvr := GetAgenticSessionV1Alpha1Resource()
2808+
if _, err := reqDyn.Resource(gvr).Namespace(project).Get(c.Request.Context(), session, v1.GetOptions{}); err != nil {
2809+
if errors.IsNotFound(err) {
2810+
c.JSON(http.StatusNotFound, gin.H{"error": "Session not found"})
2811+
return
2812+
}
2813+
log.Printf("DeleteSessionWorkspaceFile: Failed to verify session existence: %v", err)
2814+
c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to verify session"})
2815+
return
2816+
}
2817+
28162818
// Try temp service first, then regular service
28172819
serviceName := fmt.Sprintf("temp-content-%s", session)
28182820
serviceFound := false

components/frontend/package-lock.json

Lines changed: 118 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

components/frontend/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
"class-variance-authority": "^0.7.1",
2727
"clsx": "^2.1.1",
2828
"date-fns": "^4.1.0",
29+
"file-type": "^21.1.1",
2930
"highlight.js": "^11.11.1",
3031
"lucide-react": "^0.542.0",
3132
"next": "15.5.7",

components/frontend/src/app/api/projects/[name]/agentic-sessions/[sessionName]/workspace/upload/route.ts

Lines changed: 94 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { buildForwardHeadersAsync } from '@/lib/auth';
22
import { BACKEND_URL } from '@/lib/config';
33
import { NextRequest } from 'next/server';
4+
import { fileTypeFromBuffer } from 'file-type';
45

56
// Maximum file sizes based on type
67
// SDK has 1MB JSON limit, base64 adds ~33% overhead, plus JSON structure overhead
@@ -84,6 +85,57 @@ function isValidUrl(urlString: string): boolean {
8485
}
8586
}
8687

88+
// Validate file content type via magic bytes
89+
// Returns the actual MIME type detected from file content, or null if detection fails
90+
// This prevents Content-Type header spoofing attacks
91+
async function validateFileType(buffer: ArrayBuffer, claimedType: string): Promise<string> {
92+
try {
93+
// Convert ArrayBuffer to Uint8Array for file-type library
94+
const uint8Array = new Uint8Array(buffer);
95+
96+
// Detect actual file type from magic bytes
97+
const detected = await fileTypeFromBuffer(uint8Array);
98+
99+
if (!detected) {
100+
// If detection fails, treat as plain text/binary
101+
// Allow the claimed type only if it's text-based or generic binary
102+
if (claimedType.startsWith('text/') || claimedType === 'application/octet-stream') {
103+
return claimedType;
104+
}
105+
// For other types, reject if we can't verify
106+
throw new Error('Unable to verify file type. File may be corrupted or unsupported.');
107+
}
108+
109+
// Normalize both types for comparison (remove parameters like charset)
110+
const normalizedClaimed = claimedType.split(';')[0].trim().toLowerCase();
111+
const normalizedDetected = detected.mime.toLowerCase();
112+
113+
// Check if types match
114+
if (normalizedClaimed !== normalizedDetected) {
115+
// Special case: allow jpeg/jpg variations
116+
const isJpegVariant = (type: string) => type === 'image/jpeg' || type === 'image/jpg';
117+
if (isJpegVariant(normalizedClaimed) && isJpegVariant(normalizedDetected)) {
118+
return detected.mime;
119+
}
120+
121+
// Types don't match - reject
122+
throw new Error(
123+
`Content-Type mismatch: claimed '${normalizedClaimed}' but detected '${normalizedDetected}'. ` +
124+
`This may indicate a malicious file or incorrect Content-Type header.`
125+
);
126+
}
127+
128+
// Use the detected type (more trustworthy than header)
129+
return detected.mime;
130+
} catch (error) {
131+
// Re-throw validation errors
132+
if (error instanceof Error) {
133+
throw error;
134+
}
135+
throw new Error('File type validation failed');
136+
}
137+
}
138+
87139
// Compress image if it exceeds size limit
88140
// Returns compressed image buffer or original if already small enough, along with compression metadata
89141
// IMPORTANT: Compression preserves original format (PNG stays PNG, JPEG stays JPEG, etc.)
@@ -311,15 +363,33 @@ export async function POST(
311363
// Sanitize filename to prevent path traversal attacks
312364
const rawFilename = (formData.get('filename') as string) || file.name;
313365
const filename = sanitizeFilename(rawFilename);
314-
const contentType = file.type || 'application/octet-stream';
366+
const claimedContentType = file.type || 'application/octet-stream';
367+
const fileArrayBuffer = await file.arrayBuffer();
315368

316-
// Compress and validate file
369+
// Validate file type via magic bytes to prevent malicious file uploads
370+
let validatedContentType: string;
371+
try {
372+
validatedContentType = await validateFileType(fileArrayBuffer, claimedContentType);
373+
} catch (error) {
374+
return new Response(
375+
JSON.stringify({
376+
error: error instanceof Error ? error.message : 'File type validation failed',
377+
details: 'The file content does not match the claimed file type'
378+
}),
379+
{
380+
status: 400,
381+
headers: { 'Content-Type': 'application/json' },
382+
}
383+
);
384+
}
385+
386+
// Compress and validate file size
317387
let fileBuffer: ArrayBuffer;
318388
let finalContentType: string;
319389
let compressionInfo: { compressed: boolean; originalSize: number; finalSize: number };
320390

321391
try {
322-
const result = await compressAndValidate(await file.arrayBuffer(), contentType);
392+
const result = await compressAndValidate(fileArrayBuffer, validatedContentType);
323393
fileBuffer = result.buffer;
324394
finalContentType = result.contentType;
325395
compressionInfo = result.compressionInfo;
@@ -394,15 +464,33 @@ export async function POST(
394464
});
395465
}
396466

397-
const contentType = fileResp.headers.get('content-type') || 'application/octet-stream';
467+
const claimedContentType = fileResp.headers.get('content-type') || 'application/octet-stream';
468+
const fileArrayBuffer = await fileResp.arrayBuffer();
469+
470+
// Validate file type via magic bytes to prevent Content-Type spoofing
471+
let validatedContentType: string;
472+
try {
473+
validatedContentType = await validateFileType(fileArrayBuffer, claimedContentType);
474+
} catch (error) {
475+
return new Response(
476+
JSON.stringify({
477+
error: error instanceof Error ? error.message : 'File type validation failed',
478+
details: 'The file content does not match the claimed Content-Type header'
479+
}),
480+
{
481+
status: 400,
482+
headers: { 'Content-Type': 'application/json' },
483+
}
484+
);
485+
}
398486

399-
// Compress and validate file
487+
// Compress and validate file size
400488
let fileBuffer: ArrayBuffer;
401489
let finalContentType: string;
402490
let compressionInfo: { compressed: boolean; originalSize: number; finalSize: number };
403491

404492
try {
405-
const result = await compressAndValidate(await fileResp.arrayBuffer(), contentType);
493+
const result = await compressAndValidate(fileArrayBuffer, validatedContentType);
406494
fileBuffer = result.buffer;
407495
finalContentType = result.contentType;
408496
compressionInfo = result.compressionInfo;

0 commit comments

Comments
 (0)