-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Fix permissions for serverless functions #6555
Conversation
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.
PR Summary
This pull request addresses permissions issues for serverless functions and improves file access handling. Key changes include:
- Introduced ServerlessFunctionInterceptor to add signed tokens to sourceCodeFullPath, enhancing authentication for function details access
- Updated file path structure in ServerlessFunctionService to include workspace IDs, improving isolation and organization of serverless functions
- Modified FileController to provide more specific error messages for unauthorized access, aiding in debugging
- Added new environment variables (SERVERLESS_TYPE, SERVERLESS_LAMBDA_REGION, SERVERLESS_LAMBDA_ROLE) for expanded serverless capabilities
- Applied ServerlessFunctionInterceptor to key mutation methods in ServerlessFunctionResolver, enhancing security and data processing
7 file(s) reviewed, 4 comment(s)
Edit PR Review Bot Settings
); | ||
} | ||
|
||
private async processItem(item: any): Promise<any> { |
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.
style: Add type annotation for 'item' parameter
import { SortDirection } from '@ptc-org/nestjs-query-core'; | ||
import { | ||
NestjsQueryGraphQLModule, | ||
PagingStrategies, | ||
} from '@ptc-org/nestjs-query-graphql'; | ||
import { SortDirection } from '@ptc-org/nestjs-query-core'; | ||
import { NestjsQueryTypeOrmModule } from '@ptc-org/nestjs-query-typeorm'; |
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.
style: Consider grouping related imports together for better organization
const sourceCodeFullPath = | ||
fileFolderWithoutWorkspace + '/' + SOURCE_FILE_NAME; |
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.
logic: The sourceCodeFullPath now uses fileFolderWithoutWorkspace, which might cause issues if other parts of the system expect the full path including the workspace
<ArticleTable options={[ | ||
['SERVERLESS_TYPE', 'local', "Functions can either be executed through Lambda or directly by the main node process"], | ||
['SERVERLESS_LAMBDA_REGION', 'us-east-1', 'If you use the Lambda driver, region of the Lambda function'], | ||
['SERVERLESS_LAMBDA_ROLE', 'arn:aws:iam::121334:role/lambda-execution-role', "If you use the Lambda drive, name of the IAM role with the right permissions"], |
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.
syntax: Typo in 'Lambda drive', should be 'Lambda driver'
Fixes #6525
(@martmull fyi it was not related to AWS but linked to the fact that we recently enforced passing a token to access files)