-
Notifications
You must be signed in to change notification settings - Fork 3.1k
fix(copilot): fix image auth #1841
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
There was a problem hiding this 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
Fixed copilot image authorization by ensuring uploaded files include the context prefix in their storage keys and correcting the authorization logic to prevent misidentification of copilot files as execution files.
Key Changes
- Upload key generation (
upload/route.ts): Copilot, chat, and profile-picture uploads now generate storage keys with the context prefix (e.g.,copilot/timestamp-filename) usingpreserveKeyandcustomKeyoptions, ensuringinferContextFromKeycan correctly identify the file context - Authorization logic fix (
authorization.ts): Modified execution file check to only run when context is not explicitly provided (!context && isExecutionFile(...)), preventing copilot files from being incorrectly routed to execution file verification - Enhanced error logging (
authorization.ts): Improved error logging across all authorization functions to include error messages, stack traces, and error types for better debugging
Root Cause
Before this fix, copilot image uploads were failing authorization because:
- Storage keys were generated without the
copilot/prefix - The authorization flow couldn't infer the correct context from the key
- Files were incorrectly being checked as execution files, leading to authorization failures
Confidence Score: 4/5
- Safe to merge after removing the debug
console.logstatement - The core authorization fix is well-implemented and addresses a real bug. The storage key generation now correctly prefixes keys with the context, and the authorization logic properly checks for explicit context. However, the PR contains a
console.logdebug statement that must be removed before merging. apps/sim/app/api/copilot/chat/route.ts- Remove debugconsole.logon line 328
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| apps/sim/app/api/copilot/chat/route.ts | 4/5 | Added debug console.log statement that should be removed before merging |
| apps/sim/app/api/files/authorization.ts | 5/5 | Fixed context checking logic for execution files and improved error logging throughout authorization functions |
| apps/sim/app/api/files/upload/route.ts | 5/5 | Fixed storage key generation for copilot/chat/profile-pictures uploads to include context prefix and timestamp |
Sequence Diagram
sequenceDiagram
participant Client
participant UploadAPI as /api/files/upload
participant StorageService
participant ChatAPI as /api/copilot/chat
participant CopilotFiles
participant AuthZ as authorization.ts
participant Database
Note over Client,Database: Copilot Image Upload Flow
Client->>UploadAPI: POST file (context=copilot)
UploadAPI->>UploadAPI: Generate storageKey<br/>(copilot/timestamp-filename)
UploadAPI->>StorageService: uploadFile(storageKey, metadata)
StorageService->>Database: Store file metadata (userId)
StorageService-->>UploadAPI: fileInfo with key
UploadAPI-->>Client: Return file path & key
Note over Client,Database: Copilot Chat with Image Attachment
Client->>ChatAPI: POST message with fileAttachments
ChatAPI->>CopilotFiles: processCopilotAttachments(attachments)
CopilotFiles->>AuthZ: verifyFileAccess(key, userId, context='copilot')
alt Context explicitly provided
AuthZ->>AuthZ: inferredContext = 'copilot'
AuthZ->>AuthZ: Skip execution file check
else No context (legacy)
AuthZ->>AuthZ: Check if execution file
end
AuthZ->>Database: getFileMetadataByKey(key, 'copilot')
Database-->>AuthZ: fileRecord with userId
AuthZ->>AuthZ: Verify userId matches
AuthZ-->>CopilotFiles: Access granted
CopilotFiles->>StorageService: downloadFile(key)
StorageService-->>CopilotFiles: File buffer
CopilotFiles-->>ChatAPI: Processed attachments
ChatAPI->>ChatAPI: Create file content (base64)
ChatAPI-->>Client: Stream response with attachments
3 files reviewed, 1 comment
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
* Fix copilot image auth * Lint t st * Remove extra loggign * Update apps/sim/app/api/copilot/chat/route.ts Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com> --------- Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
* Fix copilot image auth * Lint * Remove extra loggign * Update apps/sim/app/api/copilot/chat/route.ts Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com> --------- Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
* Fix copilot image auth * Lint * Remove extra loggign * Update apps/sim/app/api/copilot/chat/route.ts Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com> --------- Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Summary
Fixes copilot image auth
Type of Change
Testing
Manual
Checklist