Skip to content

Conversation

@waleedlatif1
Copy link
Collaborator

@waleedlatif1 waleedlatif1 commented Oct 30, 2025

Summary

  • added file manager table, enforce permissions for viewing files
  • confirm that we are not generating presigned URLs, but lasting URLs that can only be viewed within the sim context (internal auth, explicit API keys for identification, session context)

Type of Change

  • New feature

Testing

Tested extensively.

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@vercel
Copy link

vercel bot commented Oct 30, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Preview Comments Updated (UTC)
docs Skipped Skipped Oct 30, 2025 4:46am

@waleedlatif1 waleedlatif1 marked this pull request as ready for review October 30, 2025 04:49
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

Greptile Overview

Greptile Summary

This PR implements a comprehensive file management system with proper authorization and permission enforcement. The key architectural change is replacing temporary presigned download URLs with persistent authenticated serve URLs.

Major Changes

  • New workspace_files table: Centralized metadata storage tracking file ownership, workspace association, and context (workspace, copilot, chat, knowledge-base, execution)
  • Authenticated serve URLs: Files are now accessed via /api/files/serve/[key]?context=workspace which enforces authentication and authorization on every request, rather than using time-limited presigned URLs
  • Multi-tier authorization system: apps/sim/app/api/files/authorization.ts implements comprehensive permission checks with fallback strategies (database → metadata → path patterns) for different file contexts
  • Hybrid authentication: Supports three auth methods (session cookies, API keys, internal JWT) enabling flexible access patterns for web UI, API clients, and internal services
  • Unified file managers: Context-specific managers (workspace, copilot, execution, chat) now consistently insert metadata into workspace_files table

Security Model Confirmation

The PR correctly implements lasting URLs with authorization rather than presigned URLs:

  • ✅ Presigned URLs are only generated for uploads (to enable direct client-to-storage transfers)
  • ✅ File access uses authenticated serve endpoints that verify permissions on each request
  • ✅ URLs require valid session/API key/internal JWT token to access
  • ✅ Authorization checks workspace membership before serving files

This approach provides better security than presigned URLs since access can be revoked immediately by changing permissions, rather than waiting for URL expiration.

Confidence Score: 4/5

  • This PR is generally safe to merge with proper testing of the authorization system
  • The implementation is solid with comprehensive authorization checks and proper database schema. Score is 4/5 rather than 5/5 due to the scale of changes (75 files) and complexity of the authorization fallback logic which requires thorough testing to ensure no edge cases allow unauthorized access. The multi-tier fallback system in authorization.ts is well-designed but complex.
  • apps/sim/app/api/files/authorization.ts requires careful testing of all authorization paths, especially the fallback logic for different file contexts

Important Files Changed

File Analysis

Filename Score Overview
packages/db/schema.ts 5/5 Added workspaceFiles table schema with proper relationships to user and workspace tables
apps/sim/app/api/files/authorization.ts 4/5 Comprehensive authorization system with multi-tier fallback for different file contexts; checks database, metadata, and path patterns
apps/sim/app/api/files/serve/[...path]/route.ts 5/5 File serving endpoint enforces authorization via checkHybridAuth and verifyFileAccess before streaming content
apps/sim/lib/uploads/server/metadata.ts 5/5 Database operations for file metadata with proper duplicate handling and context-based queries
apps/sim/lib/uploads/contexts/workspace/workspace-file-manager.ts 4/5 Migrated from presigned URLs to authenticated serve URLs; inserts metadata for both cloud and local storage

Sequence Diagram

sequenceDiagram
    participant Client
    participant ServeAPI as /api/files/serve
    participant HybridAuth
    participant Authorization as File Authorization
    participant MetadataDB as workspace_files Table
    participant Storage as Cloud/Local Storage
    
    Note over Client,Storage: File Upload Flow
    Client->>ServeAPI: Upload file with session/API key
    ServeAPI->>HybridAuth: Verify authentication
    HybridAuth-->>ServeAPI: userId
    ServeAPI->>Storage: Store file with metadata
    Storage->>MetadataDB: Insert file record (key, userId, workspaceId, context)
    Storage-->>ServeAPI: Return storage key
    ServeAPI-->>Client: Return authenticated serve URL (/api/files/serve/key?context=workspace)
    
    Note over Client,Storage: File Access Flow
    Client->>ServeAPI: GET /api/files/serve/[key]?context=workspace
    ServeAPI->>HybridAuth: Check auth (session/API key/internal JWT)
    HybridAuth-->>ServeAPI: userId
    ServeAPI->>Authorization: verifyFileAccess(key, userId, context)
    Authorization->>MetadataDB: Query workspace_files by key
    MetadataDB-->>Authorization: Return file record with workspaceId
    Authorization->>Authorization: Check workspace permissions
    Authorization-->>ServeAPI: Access granted/denied
    alt Access Granted
        ServeAPI->>Storage: Download file by key
        Storage-->>ServeAPI: File buffer
        ServeAPI-->>Client: Stream file content
    else Access Denied
        ServeAPI-->>Client: 401 Unauthorized
    end
Loading

74 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@waleedlatif1 waleedlatif1 merged commit 48f520b into staging Oct 30, 2025
9 checks passed
@waleedlatif1 waleedlatif1 deleted the fix/file branch October 30, 2025 05:03
waleedlatif1 added a commit that referenced this pull request Nov 12, 2025
…g files (#1766)

* feat(files): added file manager table, enforce permissions for viewing files

* rename types

* cleanup

* cleanup

* confirm local file system works with all contexts

* clean

* remove isAsync

* ignore expiresAt

* add relative imports instead of absolute ones

* absl imports

* remove redundant comments
@waleedlatif1 waleedlatif1 mentioned this pull request Nov 12, 2025
10 tasks
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