-
Notifications
You must be signed in to change notification settings - Fork 33
release: 8.2536.104 #2023
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
release: 8.2536.104 #2023
Changes from all commits
8ce1d5f
1ed3596
ac564b2
9d9df20
8180dac
2cedaeb
a9fd083
4a4b436
a254a39
981b2d7
81a6302
2c1d113
5477552
936bde6
58dbe9a
6bc97d3
f7c6541
0a1962c
769f5a1
4e6149d
113d3ac
96ad81a
0c295a8
3cefcf6
504c37e
8e8a92f
d7d5a9d
b86df22
5b2fb4c
223866b
5bd7c6a
6b96f15
2e252a7
8b291b4
dc2fe06
89070c8
7fb72c8
e0bc78d
e2fc731
7f293c0
d093f3c
2e71ed0
bf433a0
80cdba6
a4b8115
08042aa
e559297
7f9710d
aad347f
28e3da2
db6f36a
988c69d
f4f43af
9476928
3b40f80
c377eeb
9b35ce4
adb50b2
6bada64
16f6cb1
d06e988
36a51a1
4e4fae0
d079840
fe0dffc
d08e5f5
4352fcf
6cd7faf
3ec4113
96a15d8
7c4700b
dd499e8
a29f8bf
ed54985
69ef27d
783fd4b
cab7e95
dfc3e71
0e57021
e912272
eb8887d
edfc4e7
e40909d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
22.18.0 | ||
22.19.0 |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,7 +14,8 @@ | |
* under the License. | ||
*/ | ||
import { | ||
ListBucketsCommand, | ||
GetObjectCommand, | ||
ListObjectsCommand, | ||
PutObjectCommand, | ||
S3Client, | ||
} from '@aws-sdk/client-s3'; | ||
|
@@ -26,6 +27,7 @@ import { Transactional } from 'typeorm-transactional'; | |
import { OpensearchRepository } from '@/common/repositories'; | ||
import { ProjectService } from '@/domains/admin/project/project/project.service'; | ||
import type { | ||
CreateImageDownloadUrlDto, | ||
CreateImageUploadUrlDto, | ||
ImageUploadUrlTestDto, | ||
} from '../../feedback/dtos'; | ||
|
@@ -130,16 +132,34 @@ export class ChannelService { | |
return await getSignedUrl(s3, command, { expiresIn: 60 * 60 }); | ||
} | ||
|
||
async createImageDownloadUrl(dto: CreateImageDownloadUrlDto) { | ||
const { accessKeyId, secretAccessKey, endpoint, region, bucket, imageKey } = | ||
dto; | ||
|
||
const s3 = new S3Client({ | ||
credentials: { accessKeyId, secretAccessKey }, | ||
endpoint, | ||
region, | ||
}); | ||
|
||
const command = new GetObjectCommand({ | ||
Bucket: bucket, | ||
Key: imageKey, | ||
}); | ||
|
||
return await getSignedUrl(s3, command, { expiresIn: 60 }); | ||
} | ||
Comment on lines
+135
to
+151
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Presigned download URL OK; consider configurable expiry and endpoint path-style. Make expiry configurable and support path-style for MinIO/compatible endpoints via config. - const s3 = new S3Client({
- credentials: { accessKeyId, secretAccessKey },
- endpoint,
- region,
- });
+ const s3 = new S3Client({
+ credentials: { accessKeyId, secretAccessKey },
+ endpoint,
+ region,
+ // e.g., set via env S3_FORCE_PATH_STYLE=true for non-AWS endpoints
+ forcePathStyle: Boolean(this.configService.get('s3.forcePathStyle')),
+ });
@@
- return await getSignedUrl(s3, command, { expiresIn: 60 });
+ const expiresIn =
+ Number(this.configService.get('s3.downloadUrlExpiresInSeconds')) || 60;
+ return await getSignedUrl(s3, command, { expiresIn }); Additionally, the existing upload presign above includes ContentType and ACL headers that can cause issues/security exposure:
Recommend removing both unless explicitly required by policy. - const command = new PutObjectCommand({
- Bucket: bucket,
- Key: `${projectId}_${channelId}_${Date.now()}.${extension}`,
- ContentType: 'image/*',
- ACL: 'public-read',
- });
+ const command = new PutObjectCommand({
+ Bucket: bucket,
+ Key: `${projectId}_${channelId}_${Date.now()}.${extension}`,
+ }); |
||
|
||
async isValidImageConfig(dto: ImageUploadUrlTestDto) { | ||
const { accessKeyId, secretAccessKey, endpoint, region } = dto; | ||
const { accessKeyId, secretAccessKey, endpoint, region, bucket } = dto; | ||
|
||
const s3 = new S3Client({ | ||
credentials: { accessKeyId, secretAccessKey }, | ||
endpoint, | ||
region, | ||
}); | ||
|
||
const command = new ListBucketsCommand({}); | ||
const command = new ListObjectsCommand({ Bucket: bucket }); | ||
|
||
try { | ||
await s3.send(command); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,7 +14,7 @@ | |
* under the License. | ||
*/ | ||
import { ApiProperty } from '@nestjs/swagger'; | ||
import { IsString } from 'class-validator'; | ||
import { IsBoolean, IsString } from 'class-validator'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Validation bug: optional field will fail without @IsOptional and TS optional enablePresignedUrlDownload is documented as not required but will fail validation when omitted. Make it truly optional. -import { IsBoolean, IsString } from 'class-validator';
+import { IsBoolean, IsOptional, IsString } from 'class-validator';
@@
- @ApiProperty({ required: false })
- @IsBoolean()
- enablePresignedUrlDownload: boolean;
+ @ApiProperty({ required: false })
+ @IsOptional()
+ @IsBoolean()
+ enablePresignedUrlDownload?: boolean; Also applies to: 44-47 🤖 Prompt for AI Agents
|
||
|
||
export class ImageConfigRequestDto { | ||
@ApiProperty() | ||
|
@@ -40,4 +40,8 @@ export class ImageConfigRequestDto { | |
@ApiProperty({ nullable: true, type: [String] }) | ||
@IsString({ each: true }) | ||
domainWhiteList: string[]; | ||
|
||
@ApiProperty({ required: false }) | ||
@IsBoolean() | ||
enablePresignedUrlDownload: boolean; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
/** | ||
* Copyright 2025 LY Corporation | ||
* | ||
* LY Corporation licenses this file to you under the Apache License, | ||
* version 2.0 (the "License"); you may not use this file except in compliance | ||
* with the License. You may obtain a copy of the License at: | ||
* | ||
* https://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT | ||
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the | ||
* License for the specific language governing permissions and limitations | ||
* under the License. | ||
*/ | ||
|
||
export class CreateImageDownloadUrlDto { | ||
accessKeyId: string; | ||
secretAccessKey: string; | ||
endpoint: string; | ||
region: string; | ||
bucket: string; | ||
imageKey: string; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,4 +19,5 @@ export class ImageUploadUrlTestDto { | |
secretAccessKey: string; | ||
endpoint: string; | ||
region: string; | ||
bucket: string; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Add validation and Swagger metadata for the new bucket field (and consider marking secrets writeOnly). To keep DTOs consistent and avoid leaking secrets in logs/docs, annotate fields. At minimum, add decorators for bucket; optionally add writeOnly for secretAccessKey across this DTO. Apply locally within this file: export class ImageUploadUrlTestDto {
accessKeyId: string;
secretAccessKey: string;
endpoint: string;
region: string;
- bucket: string;
+ // S3 bucket name for the test upload
+ bucket: string;
} And add these imports and decorators (outside the shown range) for stronger typing and API docs: import { ApiProperty } from '@nestjs/swagger';
import { IsString, IsNotEmpty } from 'class-validator';
export class ImageUploadUrlTestDto {
@ApiProperty({ description: 'S3 access key ID', writeOnly: true })
@IsString() @IsNotEmpty()
accessKeyId: string;
@ApiProperty({ description: 'S3 secret access key', writeOnly: true })
@IsString() @IsNotEmpty()
secretAccessKey: string;
@ApiProperty({ description: 'S3-compatible endpoint (https://...)' })
@IsString() @IsNotEmpty()
endpoint: string;
@ApiProperty({ description: 'AWS region (e.g., ap-northeast-2)' })
@IsString() @IsNotEmpty()
region: string;
@ApiProperty({ description: 'Bucket name' })
@IsString() @IsNotEmpty()
bucket: string;
} Also ensure request logging (if any) masks secretAccessKey. 🤖 Prompt for AI Agents
|
||
} |
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.
Scope imageKey to a per-channel prefix to prevent cross-object access within shared buckets.
If the bucket is shared across channels/projects, allowing arbitrary
imageKey
enables presigning unrelated objects. Validate thatimageKey
starts with a channel/project-scoped prefix (e.g.,projects/${projectId}/channels/${channelId}/
). If you don’t have a prefix today, consider adding one to your image config and enforce it here.I can propose a small schema addition (e.g.,
downloadKeyPrefix
) toimageConfig
and wire validation here—want me to open a follow-up?🤖 Prompt for AI Agents