Skip to content

Commit c267b0b

Browse files
vklm2002claude
andcommitted
fix(gcs): trim whitespace from bucket_override input
Address review feedback: the manual bucket override is free-form text and users often copy-paste bucket names, which can introduce leading/trailing whitespace (spaces, newlines) and cause hard-to-debug GCS API errors (400/404). Trim bucket_override before use; a whitespace-only value trims to "" and falls back to the dropdown bucket. Tighten the test helper's bucket assertion to an exact match (calledWith) so the trimming is actually verified, and add tests for the trimmed and whitespace-only-fallback cases. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 7861007 commit c267b0b

2 files changed

Lines changed: 44 additions & 2 deletions

File tree

src/actions/google/gcs/google_cloud_storage.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ export class GoogleCloudStorageAction extends Hub.Action {
4545
// The manual override takes precedence over the bucket selected from the dropdown. This allows
4646
// writing to a bucket in another project (e.g. a partner bucket) where the service account only
4747
// has object-level access (storage.objects.create) and cannot list buckets at the project level.
48-
const bucket = request.formParams.bucket_override || request.formParams.bucket
48+
const bucket = request.formParams.bucket_override?.trim() || request.formParams.bucket
4949

5050
if (!bucket) {
5151
const error: Error = errorWith(

src/actions/google/gcs/test_google_cloud_storage.ts

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,9 @@ function expectGoogleCloudStorageMatch(request: Hub.ActionRequest,
6969
// so stubbing it again would throw. Such callers pass stubNow = false.
7070
const stubDate = stubNow ? sinon.stub(Date, "now").callsFake(() => 1234) : undefined
7171
return chai.expect(action.validateAndExecute(request)).to.be.fulfilled.then(() => {
72-
chai.expect(bucketSpy).to.have.been.calledWithMatch(bucketMatch)
72+
// Exact match (not calledWithMatch, which does substring matching for strings) so that
73+
// whitespace-trimming assertions on the bucket name are meaningful.
74+
chai.expect(bucketSpy).to.have.been.calledWith(bucketMatch)
7375
chai.expect(fileSpy).to.have.been.calledWithMatch(fileMatch)
7476
stubClient.restore()
7577
stubSuggestedFilename.restore()
@@ -430,6 +432,46 @@ describe(`${action.constructor.name} unit tests`, () => {
430432
"mywackyfilename",
431433
Buffer.from("1,2,3,4", "utf8"))
432434
})
435+
436+
it("trims whitespace from the bucket_override value", () => {
437+
const request = new Hub.ActionRequest()
438+
request.type = Hub.ActionType.Dashboard
439+
request.params = {
440+
client_email: "myemail",
441+
private_key: "mykey",
442+
project_id: "myproject",
443+
}
444+
request.formParams = {
445+
bucket: "listedbucket",
446+
bucket_override: " partner-bucket\n",
447+
filename: "mywackyfilename",
448+
}
449+
request.attachment = {dataBuffer: Buffer.from("1,2,3,4", "utf8")}
450+
return expectGoogleCloudStorageMatch(request,
451+
"partner-bucket",
452+
"mywackyfilename",
453+
Buffer.from("1,2,3,4", "utf8"))
454+
})
455+
456+
it("falls back to the dropdown bucket when bucket_override is only whitespace", () => {
457+
const request = new Hub.ActionRequest()
458+
request.type = Hub.ActionType.Dashboard
459+
request.params = {
460+
client_email: "myemail",
461+
private_key: "mykey",
462+
project_id: "myproject",
463+
}
464+
request.formParams = {
465+
bucket: "listedbucket",
466+
bucket_override: " ",
467+
filename: "mywackyfilename",
468+
}
469+
request.attachment = {dataBuffer: Buffer.from("1,2,3,4", "utf8")}
470+
return expectGoogleCloudStorageMatch(request,
471+
"listedbucket",
472+
"mywackyfilename",
473+
Buffer.from("1,2,3,4", "utf8"))
474+
})
433475
})
434476

435477
describe("form", () => {

0 commit comments

Comments
 (0)