From c1aa7d85d6a3d72a4390dffef579b9df27d50917 Mon Sep 17 00:00:00 2001 From: "gcf-owl-bot[bot]" <78513119+gcf-owl-bot[bot]@users.noreply.github.com> Date: Wed, 17 Sep 2025 14:36:25 -0400 Subject: [PATCH 1/7] build: configure release builds using multi-scm (#2552) Source-Link: https://github.com/googleapis/synthtool/commit/8200f932b0f019bb4679adab68776a7ac8c3cc45 Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-nodejs:latest@sha256:22e41dd7cd82683fa338b647abcc3a29ddb1b17e800b089adc0bec0a3175312c Co-authored-by: Owl Bot Co-authored-by: Denis DelGrosso <85250797+ddelgrosso1@users.noreply.github.com> --- .github/.OwlBot.lock.yaml | 4 ++-- .github/release-trigger.yml | 1 + .github/workflows/ci.yaml | 19 +++++++++-------- .kokoro/common.cfg | 2 +- .kokoro/continuous/node18/common.cfg | 24 ++++++++++++++++++++++ .kokoro/continuous/node18/lint.cfg | 4 ++++ .kokoro/continuous/node18/samples-test.cfg | 12 +++++++++++ .kokoro/continuous/node18/system-test.cfg | 12 +++++++++++ .kokoro/continuous/node18/test.cfg | 0 .kokoro/presubmit/node18/common.cfg | 24 ++++++++++++++++++++++ .kokoro/presubmit/node18/samples-test.cfg | 12 +++++++++++ .kokoro/presubmit/node18/system-test.cfg | 12 +++++++++++ .kokoro/presubmit/node18/test.cfg | 0 .kokoro/release/docs-devsite.cfg | 2 +- .kokoro/release/docs.cfg | 2 +- .kokoro/release/docs.sh | 2 +- .kokoro/samples-test.sh | 2 +- .kokoro/test.bat | 2 +- .kokoro/test.sh | 2 +- .kokoro/trampoline_v2.sh | 2 +- 20 files changed, 121 insertions(+), 19 deletions(-) create mode 100644 .kokoro/continuous/node18/common.cfg create mode 100644 .kokoro/continuous/node18/lint.cfg create mode 100644 .kokoro/continuous/node18/samples-test.cfg create mode 100644 .kokoro/continuous/node18/system-test.cfg create mode 100644 .kokoro/continuous/node18/test.cfg create mode 100644 .kokoro/presubmit/node18/common.cfg create mode 100644 .kokoro/presubmit/node18/samples-test.cfg create mode 100644 .kokoro/presubmit/node18/system-test.cfg create mode 100644 .kokoro/presubmit/node18/test.cfg diff --git a/.github/.OwlBot.lock.yaml b/.github/.OwlBot.lock.yaml index 24943e116..1cab9caf1 100644 --- a/.github/.OwlBot.lock.yaml +++ b/.github/.OwlBot.lock.yaml @@ -13,5 +13,5 @@ # limitations under the License. docker: image: gcr.io/cloud-devrel-public-resources/owlbot-nodejs:latest - digest: sha256:609822e3c09b7a1bd90b99655904609f162cc15acb4704f1edf778284c36f429 -# created: 2024-10-01T19:34:30.797530443Z + digest: sha256:22e41dd7cd82683fa338b647abcc3a29ddb1b17e800b089adc0bec0a3175312c +# created: 2024-10-30T16:51:59.982020867Z diff --git a/.github/release-trigger.yml b/.github/release-trigger.yml index d4ca94189..1e3188f69 100644 --- a/.github/release-trigger.yml +++ b/.github/release-trigger.yml @@ -1 +1,2 @@ enabled: true +multiScmName: nodejs-storage \ No newline at end of file diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 18ec4115b..a68801224 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -9,10 +9,10 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - node: [14, 16, 18, 20] + node: [14, 16, 18] steps: - - uses: actions/checkout@v3 - - uses: actions/setup-node@v3 + - uses: actions/checkout@v4 + - uses: actions/setup-node@v4 with: node-version: ${{ matrix.node }} - run: node --version @@ -29,10 +29,10 @@ jobs: windows: runs-on: windows-latest steps: - - uses: actions/checkout@v3 - - uses: actions/setup-node@v3 + - uses: actions/checkout@v4 + - uses: actions/setup-node@v4 with: - node-version: 14 + node-version: 18 - run: npm install --engine-strict - run: npm test env: @@ -40,9 +40,10 @@ jobs: lint: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v3 - - uses: actions/setup-node@v3 + - uses: actions/checkout@v4 + - uses: actions/setup-node@v4 with: - node-version: 14 + node-version: 18 - run: npm install - run: npm run lint + diff --git a/.kokoro/common.cfg b/.kokoro/common.cfg index 4ee7584a3..713afb2ec 100644 --- a/.kokoro/common.cfg +++ b/.kokoro/common.cfg @@ -16,7 +16,7 @@ build_file: "nodejs-storage/.kokoro/trampoline_v2.sh" # Configure the docker image for kokoro-trampoline. env_vars: { key: "TRAMPOLINE_IMAGE" - value: "gcr.io/cloud-devrel-kokoro-resources/node:14-user" + value: "gcr.io/cloud-devrel-kokoro-resources/node:18-user" } env_vars: { key: "TRAMPOLINE_BUILD_FILE" diff --git a/.kokoro/continuous/node18/common.cfg b/.kokoro/continuous/node18/common.cfg new file mode 100644 index 000000000..713afb2ec --- /dev/null +++ b/.kokoro/continuous/node18/common.cfg @@ -0,0 +1,24 @@ +# Format: //devtools/kokoro/config/proto/build.proto + +# Build logs will be here +action { + define_artifacts { + regex: "**/*sponge_log.xml" + } +} + +# Download trampoline resources. +gfile_resources: "/bigstore/cloud-devrel-kokoro-resources/trampoline" + +# Use the trampoline script to run in docker. +build_file: "nodejs-storage/.kokoro/trampoline_v2.sh" + +# Configure the docker image for kokoro-trampoline. +env_vars: { + key: "TRAMPOLINE_IMAGE" + value: "gcr.io/cloud-devrel-kokoro-resources/node:18-user" +} +env_vars: { + key: "TRAMPOLINE_BUILD_FILE" + value: "github/nodejs-storage/.kokoro/test.sh" +} diff --git a/.kokoro/continuous/node18/lint.cfg b/.kokoro/continuous/node18/lint.cfg new file mode 100644 index 000000000..72c4ad241 --- /dev/null +++ b/.kokoro/continuous/node18/lint.cfg @@ -0,0 +1,4 @@ +env_vars: { + key: "TRAMPOLINE_BUILD_FILE" + value: "github/nodejs-storage/.kokoro/lint.sh" +} diff --git a/.kokoro/continuous/node18/samples-test.cfg b/.kokoro/continuous/node18/samples-test.cfg new file mode 100644 index 000000000..2137cefc0 --- /dev/null +++ b/.kokoro/continuous/node18/samples-test.cfg @@ -0,0 +1,12 @@ +# Download resources for system tests (service account key, etc.) +gfile_resources: "/bigstore/cloud-devrel-kokoro-resources/google-cloud-nodejs" + +env_vars: { + key: "TRAMPOLINE_BUILD_FILE" + value: "github/nodejs-storage/.kokoro/samples-test.sh" +} + +env_vars: { + key: "SECRET_MANAGER_KEYS" + value: "long-door-651-kokoro-system-test-service-account" +} \ No newline at end of file diff --git a/.kokoro/continuous/node18/system-test.cfg b/.kokoro/continuous/node18/system-test.cfg new file mode 100644 index 000000000..9ba9ad1c7 --- /dev/null +++ b/.kokoro/continuous/node18/system-test.cfg @@ -0,0 +1,12 @@ +# Download resources for system tests (service account key, etc.) +gfile_resources: "/bigstore/cloud-devrel-kokoro-resources/google-cloud-nodejs" + +env_vars: { + key: "TRAMPOLINE_BUILD_FILE" + value: "github/nodejs-storage/.kokoro/system-test.sh" +} + +env_vars: { + key: "SECRET_MANAGER_KEYS" + value: "long-door-651-kokoro-system-test-service-account" +} \ No newline at end of file diff --git a/.kokoro/continuous/node18/test.cfg b/.kokoro/continuous/node18/test.cfg new file mode 100644 index 000000000..e69de29bb diff --git a/.kokoro/presubmit/node18/common.cfg b/.kokoro/presubmit/node18/common.cfg new file mode 100644 index 000000000..713afb2ec --- /dev/null +++ b/.kokoro/presubmit/node18/common.cfg @@ -0,0 +1,24 @@ +# Format: //devtools/kokoro/config/proto/build.proto + +# Build logs will be here +action { + define_artifacts { + regex: "**/*sponge_log.xml" + } +} + +# Download trampoline resources. +gfile_resources: "/bigstore/cloud-devrel-kokoro-resources/trampoline" + +# Use the trampoline script to run in docker. +build_file: "nodejs-storage/.kokoro/trampoline_v2.sh" + +# Configure the docker image for kokoro-trampoline. +env_vars: { + key: "TRAMPOLINE_IMAGE" + value: "gcr.io/cloud-devrel-kokoro-resources/node:18-user" +} +env_vars: { + key: "TRAMPOLINE_BUILD_FILE" + value: "github/nodejs-storage/.kokoro/test.sh" +} diff --git a/.kokoro/presubmit/node18/samples-test.cfg b/.kokoro/presubmit/node18/samples-test.cfg new file mode 100644 index 000000000..2137cefc0 --- /dev/null +++ b/.kokoro/presubmit/node18/samples-test.cfg @@ -0,0 +1,12 @@ +# Download resources for system tests (service account key, etc.) +gfile_resources: "/bigstore/cloud-devrel-kokoro-resources/google-cloud-nodejs" + +env_vars: { + key: "TRAMPOLINE_BUILD_FILE" + value: "github/nodejs-storage/.kokoro/samples-test.sh" +} + +env_vars: { + key: "SECRET_MANAGER_KEYS" + value: "long-door-651-kokoro-system-test-service-account" +} \ No newline at end of file diff --git a/.kokoro/presubmit/node18/system-test.cfg b/.kokoro/presubmit/node18/system-test.cfg new file mode 100644 index 000000000..9ba9ad1c7 --- /dev/null +++ b/.kokoro/presubmit/node18/system-test.cfg @@ -0,0 +1,12 @@ +# Download resources for system tests (service account key, etc.) +gfile_resources: "/bigstore/cloud-devrel-kokoro-resources/google-cloud-nodejs" + +env_vars: { + key: "TRAMPOLINE_BUILD_FILE" + value: "github/nodejs-storage/.kokoro/system-test.sh" +} + +env_vars: { + key: "SECRET_MANAGER_KEYS" + value: "long-door-651-kokoro-system-test-service-account" +} \ No newline at end of file diff --git a/.kokoro/presubmit/node18/test.cfg b/.kokoro/presubmit/node18/test.cfg new file mode 100644 index 000000000..e69de29bb diff --git a/.kokoro/release/docs-devsite.cfg b/.kokoro/release/docs-devsite.cfg index eea22f239..c98f038de 100644 --- a/.kokoro/release/docs-devsite.cfg +++ b/.kokoro/release/docs-devsite.cfg @@ -11,7 +11,7 @@ before_action { # doc publications use a Python image. env_vars: { key: "TRAMPOLINE_IMAGE" - value: "gcr.io/cloud-devrel-kokoro-resources/node:14-user" + value: "gcr.io/cloud-devrel-kokoro-resources/node:18-user" } # Download trampoline resources. diff --git a/.kokoro/release/docs.cfg b/.kokoro/release/docs.cfg index 25c377eef..71c80e6b0 100644 --- a/.kokoro/release/docs.cfg +++ b/.kokoro/release/docs.cfg @@ -11,7 +11,7 @@ before_action { # doc publications use a Python image. env_vars: { key: "TRAMPOLINE_IMAGE" - value: "gcr.io/cloud-devrel-kokoro-resources/node:14-user" + value: "gcr.io/cloud-devrel-kokoro-resources/node:18-user" } # Download trampoline resources. diff --git a/.kokoro/release/docs.sh b/.kokoro/release/docs.sh index 1d8f3f490..e9079a605 100755 --- a/.kokoro/release/docs.sh +++ b/.kokoro/release/docs.sh @@ -16,7 +16,7 @@ set -eo pipefail -# build jsdocs (Python is installed on the Node 10 docker image). +# build jsdocs (Python is installed on the Node 18 docker image). if [[ -z "$CREDENTIALS" ]]; then # if CREDENTIALS are explicitly set, assume we're testing locally # and don't set NPM_CONFIG_PREFIX. diff --git a/.kokoro/samples-test.sh b/.kokoro/samples-test.sh index 8c5d108cb..c1cb0fc77 100755 --- a/.kokoro/samples-test.sh +++ b/.kokoro/samples-test.sh @@ -56,7 +56,7 @@ fi # codecov combines coverage across integration and unit tests. Include # the logic below for any environment you wish to collect coverage for: -COVERAGE_NODE=14 +COVERAGE_NODE=18 if npx check-node-version@3.3.0 --silent --node $COVERAGE_NODE; then NYC_BIN=./node_modules/nyc/bin/nyc.js if [ -f "$NYC_BIN" ]; then diff --git a/.kokoro/test.bat b/.kokoro/test.bat index 0bb124052..caf825656 100644 --- a/.kokoro/test.bat +++ b/.kokoro/test.bat @@ -21,7 +21,7 @@ cd .. @rem we upgrade Node.js in the image: SET PATH=%PATH%;/cygdrive/c/Program Files/nodejs/npm -call nvm use v14.17.3 +call nvm use 18 call which node call npm install || goto :error diff --git a/.kokoro/test.sh b/.kokoro/test.sh index 862d478d3..0d9f6392a 100755 --- a/.kokoro/test.sh +++ b/.kokoro/test.sh @@ -39,7 +39,7 @@ npm test # codecov combines coverage across integration and unit tests. Include # the logic below for any environment you wish to collect coverage for: -COVERAGE_NODE=14 +COVERAGE_NODE=18 if npx check-node-version@3.3.0 --silent --node $COVERAGE_NODE; then NYC_BIN=./node_modules/nyc/bin/nyc.js if [ -f "$NYC_BIN" ]; then diff --git a/.kokoro/trampoline_v2.sh b/.kokoro/trampoline_v2.sh index 4d0311212..5d6cfcca5 100755 --- a/.kokoro/trampoline_v2.sh +++ b/.kokoro/trampoline_v2.sh @@ -44,7 +44,7 @@ # the project root. # # Here is an example for running this script. -# TRAMPOLINE_IMAGE=gcr.io/cloud-devrel-kokoro-resources/node:10-user \ +# TRAMPOLINE_IMAGE=gcr.io/cloud-devrel-kokoro-resources/node:18-user \ # TRAMPOLINE_BUILD_FILE=.kokoro/system-test.sh \ # .kokoro/trampoline_v2.sh From 4bab38928e77c3f00b5f98643568f49119e29dbd Mon Sep 17 00:00:00 2001 From: "gcf-owl-bot[bot]" <78513119+gcf-owl-bot[bot]@users.noreply.github.com> Date: Wed, 17 Sep 2025 15:44:08 -0400 Subject: [PATCH 2/7] chore: fix `npm` for Node v18 samples tests (#2557) * chore: fix `npm` for Node v18 samples tests chore: fix `npm` for samples tests Source-Link: https://github.com/googleapis/synthtool/commit/4d752428d93b18b69c28acdbd9aa821a517db73a Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-nodejs:latest@sha256:0d39e59663287ae929c1d4ccf8ebf7cef9946826c9b86eda7e85d8d752dbb584 * re-add deleted env var * ignore samples-test.sh --------- Co-authored-by: Owl Bot Co-authored-by: Denis DelGrosso <85250797+ddelgrosso1@users.noreply.github.com> Co-authored-by: Denis DelGrosso --- .github/.OwlBot.lock.yaml | 5 +++-- .github/workflows/ci.yaml | 2 +- .kokoro/samples-test.sh | 4 ++++ owlbot.py | 3 ++- 4 files changed, 10 insertions(+), 4 deletions(-) diff --git a/.github/.OwlBot.lock.yaml b/.github/.OwlBot.lock.yaml index 1cab9caf1..15cd660fe 100644 --- a/.github/.OwlBot.lock.yaml +++ b/.github/.OwlBot.lock.yaml @@ -13,5 +13,6 @@ # limitations under the License. docker: image: gcr.io/cloud-devrel-public-resources/owlbot-nodejs:latest - digest: sha256:22e41dd7cd82683fa338b647abcc3a29ddb1b17e800b089adc0bec0a3175312c -# created: 2024-10-30T16:51:59.982020867Z + digest: sha256:0d39e59663287ae929c1d4ccf8ebf7cef9946826c9b86eda7e85d8d752dbb584 +# created: 2024-11-21T22:39:44.342569463Z + diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index a68801224..1561bbc93 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -46,4 +46,4 @@ jobs: node-version: 18 - run: npm install - run: npm run lint - + diff --git a/.kokoro/samples-test.sh b/.kokoro/samples-test.sh index c1cb0fc77..d195fbf7c 100755 --- a/.kokoro/samples-test.sh +++ b/.kokoro/samples-test.sh @@ -18,6 +18,10 @@ set -eo pipefail export NPM_CONFIG_PREFIX=${HOME}/.npm-global +# Ensure the npm global directory is writable, otherwise rebuild `npm` +mkdir -p $NPM_CONFIG_PREFIX +npm config -g ls || npm i -g npm@`npm --version` + # Setup service account credentials. export GOOGLE_APPLICATION_CREDENTIALS=${KOKORO_GFILE_DIR}/secret_manager/long-door-651-kokoro-system-test-service-account export GCLOUD_PROJECT=long-door-651 diff --git a/owlbot.py b/owlbot.py index 3df451978..0607a2f0c 100644 --- a/owlbot.py +++ b/owlbot.py @@ -30,7 +30,8 @@ '.kokoro/continuous/node14/system-test.cfg', '.kokoro/presubmit/node14/system-test.cfg', '.kokoro/release/publish.cfg', - '.kokoro/system-test.sh' + '.kokoro/system-test.sh', + '.kokoro/samples-test.sh', ]) # Create .config directory under $HOME to get around permissions issues From 19f8a32ad112be8fd0347cef0954feb5a01a02f6 Mon Sep 17 00:00:00 2001 From: "gcf-owl-bot[bot]" <78513119+gcf-owl-bot[bot]@users.noreply.github.com> Date: Wed, 17 Sep 2025 16:23:00 -0400 Subject: [PATCH 3/7] chore(Node.js): Update PR Template (#2583) * chore(Node.js): Update PR Template * feat: Update PR Template * docs: Update synthtool/gcp/templates/node_library/.github/PULL_REQUEST_TEMPLATE.md Co-authored-by: sofisl <55454395+sofisl@users.noreply.github.com> * docs: Update synthtool/gcp/templates/node_library/.github/PULL_REQUEST_TEMPLATE.md Co-authored-by: sofisl <55454395+sofisl@users.noreply.github.com> --------- Co-authored-by: sofisl <55454395+sofisl@users.noreply.github.com> Source-Link: https://github.com/googleapis/synthtool/commit/bb0a3506602525f63c7002f8d13345be3678effb Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-nodejs:latest@sha256:d0befde9bb710526253d1badc2d5b02884b466acc99db4e26ce8e71e69072ea0 * fix ignore --------- Co-authored-by: Owl Bot Co-authored-by: Denis DelGrosso Co-authored-by: Denis DelGrosso <85250797+ddelgrosso1@users.noreply.github.com> --- .github/.OwlBot.lock.yaml | 6 +++--- .github/PULL_REQUEST_TEMPLATE.md | 33 +++++++++++++++++++++++++++----- .github/workflows/ci.yaml | 1 - 3 files changed, 31 insertions(+), 9 deletions(-) diff --git a/.github/.OwlBot.lock.yaml b/.github/.OwlBot.lock.yaml index 15cd660fe..d4140f443 100644 --- a/.github/.OwlBot.lock.yaml +++ b/.github/.OwlBot.lock.yaml @@ -1,4 +1,4 @@ -# Copyright 2024 Google LLC +# Copyright 2025 Google LLC # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -13,6 +13,6 @@ # limitations under the License. docker: image: gcr.io/cloud-devrel-public-resources/owlbot-nodejs:latest - digest: sha256:0d39e59663287ae929c1d4ccf8ebf7cef9946826c9b86eda7e85d8d752dbb584 -# created: 2024-11-21T22:39:44.342569463Z + digest: sha256:d0befde9bb710526253d1badc2d5b02884b466acc99db4e26ce8e71e69072ea0 +# created: 2025-03-07T03:28:55.703836867Z diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 1a639c73d..93dd20eeb 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -1,7 +1,30 @@ -Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly: -- [ ] Make sure to open an issue as a [bug/issue](https://github.com/googleapis/nodejs-storage/issues/new/choose) before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea +> Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly: + +## Description + +> Please provide a detailed description for the change. +> As much as possible, please try to keep changes separate by purpose. For example, try not to make a one-line bug fix in a feature request, or add an irrelevant README change to a bug fix. + +## Impact + +> What's the impact of this change? + +## Testing + +> Have you added unit and integration tests if necessary? +> Were any tests changed? Are any breaking changes necessary? + +## Additional Information + +> Any additional details that we should be aware of? + +## Checklist + +- [ ] Make sure to open an issue as a [bug/issue](https://github.com/googleapis/nodejs-storage/issues/new/choose) before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea - [ ] Ensure the tests and linter pass -- [ ] Code coverage does not decrease (if any source code was changed) -- [ ] Appropriate docs were updated (if necessary) +- [ ] Code coverage does not decrease +- [ ] Appropriate docs were updated +- [ ] Appropriate comments were added, particularly in complex areas or places that require background +- [ ] No new warnings or issues will be generated from this change -Fixes # 🦕 +Fixes #issue_number_goes_here 🦕 diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 1561bbc93..f547520d5 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -46,4 +46,3 @@ jobs: node-version: 18 - run: npm install - run: npm run lint - From b38b5d221f2cb72658c1eb4a726315ab395a542c Mon Sep 17 00:00:00 2001 From: Thiyagu K Date: Thu, 18 Sep 2025 18:27:34 +0530 Subject: [PATCH 4/7] fix: Common Service: should retry a request failed (#2652) * fix: Retry failed requests * fix: Add comment explaining increased timeout --- system-test/common.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/system-test/common.ts b/system-test/common.ts index 0288143c6..17bc59b61 100644 --- a/system-test/common.ts +++ b/system-test/common.ts @@ -57,7 +57,10 @@ describe('Common', () => { }); it('should retry a request', function (done) { - this.timeout(60 * 1000); + // We've increased the timeout to accommodate the retry backoff strategy. + // The test's retry attempts and the delay between them can exceed the default timeout, + // causing a false negative (test failure due to timeout instead of a logic error). + this.timeout(90 * 1000); let numRequestAttempts = 0; From 08d7abf32dd365b24ce34c66174be06c30bfce8f Mon Sep 17 00:00:00 2001 From: Thiyagu K Date: Mon, 6 Oct 2025 19:34:28 +0530 Subject: [PATCH 5/7] fix: Implement path containment to prevent traversal attacks (#2654) * fix: Implement path containment to prevent traversal attacks This patch introduces strict path validation in TransferManager.downloadManyFiles to mitigate Arbitrary File Write and Path Traversal vulnerabilities. The fix includes two layers of defense: 1. Rejects Absolute Paths: Immediately throws an error if the object name is an absolute path (e.g., /etc/passwd). 2. Containment Check: Uses path.resolve to normalize the destination path and verify it remains strictly within the intended baseDir, preventing traversal using ../ sequences. SECURITY NOTE: This changes behavior by actively rejecting files with malicious path segments that were previously susceptible to writing outside the target directory. * fix: Use path.relative for robust path traversal check * fix: Enforce GCS standard '/' for directory marker detection * fix: Secure destination path against traversal * add error message * fix: Correct download destination logic and ensure recursive directory creation This commit resolves several critical issues in the `downloadManyFiles` logic related to path handling, destination assignment, and concurrent directory creation, enabling proper execution of bulk downloads and passing relevant tests. * fix: Optimize fsp.mkdir calls using a Set in downloadManyFiles Avoids redundant file system calls (fsp.mkdir) when downloading multiple files within the same directory. The call, while idempotent, was being performed for every file download, leading to unnecessary I/O overhead. This commit introduces a to track directories that have already been created within a single call, ensuring that is executed only once per unique destination directory path. * refactor: Extract base directory initialization/validation Moves the logic for resolving and validating the base download directory (`baseDir`, including initial path traversal checks) out of `downloadManyFiles` and into the private helper `_resolveAndValidateBaseDir`. This change cleans up the primary download execution path, making the file-by-file iteration loop more focused and readable. * fix * refactor: Remove explicit .code assignment from RequestError Removes the 'SECURITY_ABSOLUTE_PATH_REJECTED' & 'SECURITY_PATH_TRAVERSAL_REJECTED' code assignment from the thrown RequestError. The corresponding test assertion is updated to check the error message and type instead of the removed .code property. --- samples/system-test/transfer-manager.test.js | 21 +++- src/file.ts | 3 + src/transfer-manager.ts | 103 ++++++++++++++++--- test/transfer-manager.ts | 98 +++++++++++++++--- 4 files changed, 192 insertions(+), 33 deletions(-) diff --git a/samples/system-test/transfer-manager.test.js b/samples/system-test/transfer-manager.test.js index f6180bed3..9f51c3598 100644 --- a/samples/system-test/transfer-manager.test.js +++ b/samples/system-test/transfer-manager.test.js @@ -56,26 +56,37 @@ describe('transfer manager', () => { ); }); - it('should download mulitple files', async () => { + it('should download multiple files', async () => { + // Remove absolute path marker to prepare for joining/validation. + const expectedFirstFilePath = firstFilePath.startsWith('/') + ? firstFilePath.slice(1) + : firstFilePath; + const expectedSecondFilePath = secondFilePath.startsWith('/') + ? secondFilePath.slice(1) + : secondFilePath; const output = execSync( - `node downloadManyFilesWithTransferManager.js ${bucketName} ${firstFilePath} ${secondFilePath}` + `node downloadManyFilesWithTransferManager.js ${bucketName} ${expectedFirstFilePath} ${expectedSecondFilePath}` ); assert.match( output, new RegExp( - `gs://${bucketName}/${firstFilePath} downloaded to ${firstFilePath}.\ngs://${bucketName}/${secondFilePath} downloaded to ${secondFilePath}.` + `gs://${bucketName}/${expectedFirstFilePath} downloaded to ${expectedFirstFilePath}.\ngs://${bucketName}/${expectedSecondFilePath} downloaded to ${expectedSecondFilePath}.` ) ); }); it('should download a file utilizing chunked download', async () => { + // Remove absolute path marker to prepare for joining/validation. + const expectedFirstFilePath = firstFilePath.startsWith('/') + ? firstFilePath.slice(1) + : firstFilePath; const output = execSync( - `node downloadFileInChunksWithTransferManager.js ${bucketName} ${firstFilePath} ${downloadFilePath} ${chunkSize}` + `node downloadFileInChunksWithTransferManager.js ${bucketName} ${expectedFirstFilePath} ${downloadFilePath} ${chunkSize}` ); assert.match( output, new RegExp( - `gs://${bucketName}/${firstFilePath} downloaded to ${downloadFilePath}.` + `gs://${bucketName}/${expectedFirstFilePath} downloaded to ${downloadFilePath}.` ) ); }); diff --git a/src/file.ts b/src/file.ts index 0d18458bb..2aa4ec7e1 100644 --- a/src/file.ts +++ b/src/file.ts @@ -545,6 +545,9 @@ export enum FileExceptionMessages { To be sure the content is the same, you should try uploading the file again.`, MD5_RESUMED_UPLOAD = 'MD5 cannot be used with a continued resumable upload as MD5 cannot be extended from an existing value', MISSING_RESUME_CRC32C_FINAL_UPLOAD = 'The CRC32C is missing for the final portion of a resumed upload, which is required for validation. Please provide `resumeCRC32C` if validation is required, or disable `validation`.', + ABSOLUTE_FILE_NAME = 'Object name is an absolute path. Security block to prevent arbitrary file writes.', + TRAVERSAL_OUTSIDE_BASE = 'Path traversal detected. Security block to prevent writing outside the base directory.', + TRAVERSAL_OUTSIDE_BASE_DESTINATION = "The provided destination path is unsafe and attempts to traverse outside the application's base directory (current working directory).", } /** diff --git a/src/transfer-manager.ts b/src/transfer-manager.ts index dd4e41eeb..db7dd4fc6 100644 --- a/src/transfer-manager.ts +++ b/src/transfer-manager.ts @@ -497,9 +497,16 @@ export class TransferManager { [GCCL_GCS_CMD_KEY]: GCCL_GCS_CMD_FEATURE.UPLOAD_MANY, }; - passThroughOptionsCopy.destination = options.customDestinationBuilder - ? options.customDestinationBuilder(filePath, options) - : filePath.split(path.sep).join(path.posix.sep); + if (options.customDestinationBuilder) { + passThroughOptionsCopy.destination = options.customDestinationBuilder( + filePath, + options + ); + } else { + let segments = filePath.split(path.sep); + segments = segments.filter(s => s !== ''); + passThroughOptionsCopy.destination = path.posix.join(...segments); + } if (options.prefix) { passThroughOptionsCopy.destination = path.posix.join( ...options.prefix.split(path.sep), @@ -587,27 +594,61 @@ export class TransferManager { }); } + const baseDir = this._resolveAndValidateBaseDir(options); + const stripRegexString = options.stripPrefix ? `^${options.stripPrefix}` : EMPTY_REGEX; const regex = new RegExp(stripRegexString, 'g'); + const createdDirectories = new Set(); for (const file of files) { + let name = file.name; + + // Apply stripPrefix first if requested + if (options.stripPrefix) { + name = name.replace(regex, ''); + } + + // This ensures the full intended relative path is validated. + if (options.prefix) { + name = path.join(options.prefix, name); + } + + // Reject absolute paths and traversal sequences + if (path.isAbsolute(name)) { + const absolutePathError = new RequestError( + FileExceptionMessages.ABSOLUTE_FILE_NAME + ); + throw absolutePathError; + } + + // Resolve the final path and perform the containment check + let finalPath = path.resolve(baseDir, name); + const normalizedBaseDir = baseDir.endsWith(path.sep) + ? baseDir + : baseDir + path.sep; + if (finalPath !== baseDir && !finalPath.startsWith(normalizedBaseDir)) { + const traversalError = new RequestError( + FileExceptionMessages.TRAVERSAL_OUTSIDE_BASE + ); + throw traversalError; + } + + if (file.name.endsWith('/') && !finalPath.endsWith(path.sep)) { + finalPath = finalPath + path.sep; + } + const passThroughOptionsCopy = { ...options.passthroughOptions, + destination: finalPath, [GCCL_GCS_CMD_KEY]: GCCL_GCS_CMD_FEATURE.DOWNLOAD_MANY, }; - if (options.prefix || passThroughOptionsCopy.destination) { - passThroughOptionsCopy.destination = path.join( - options.prefix || '', - passThroughOptionsCopy.destination || '', - file.name - ); - } - if (options.stripPrefix) { - passThroughOptionsCopy.destination = file.name.replace(regex, ''); - } + const destinationDir = finalPath.endsWith(path.sep) + ? finalPath + : path.dirname(finalPath); + if ( options.skipIfExists && existsSync(passThroughOptionsCopy.destination || '') @@ -618,8 +659,12 @@ export class TransferManager { promises.push( limit(async () => { const destination = passThroughOptionsCopy.destination; + if (!createdDirectories.has(destinationDir)) { + // If not, create it and add it to the set for tracking + await fsp.mkdir(destinationDir, {recursive: true}); + createdDirectories.add(destinationDir); + } if (destination && destination.endsWith(path.sep)) { - await fsp.mkdir(destination, {recursive: true}); return Promise.resolve([ Buffer.alloc(0), ]) as Promise; @@ -867,4 +912,34 @@ export class TransferManager { : yield fullPath; } } + + /** + * Resolves the absolute base directory for downloads and validates it against + * the current working directory (CWD) to prevent path traversal outside the base destination. + * @param options The download options, potentially containing passthroughOptions.destination. + * @returns The absolute, validated base directory path (baseDir). + */ + private _resolveAndValidateBaseDir( + options: DownloadManyFilesOptions + ): string { + const cwd = process.cwd(); + + // Resolve baseDir, defaulting to CWD if no destination is provided + const baseDir = path.resolve( + options.passthroughOptions?.destination ?? cwd + ); + + // Check for path traversal: baseDir must be equal to or contained within cwd. + const relativeBaseDir = path.relative(cwd, baseDir); + + // The condition checks for traversal ('..') or cross-drive traversal (absolute path on Windows) + if (relativeBaseDir.startsWith('..') || path.isAbsolute(relativeBaseDir)) { + const traversalError = new RequestError( + FileExceptionMessages.TRAVERSAL_OUTSIDE_BASE_DESTINATION + ); + throw traversalError; + } + + return baseDir; + } } diff --git a/test/transfer-manager.ts b/test/transfer-manager.ts index 2582782fa..2ab91ec1f 100644 --- a/test/transfer-manager.ts +++ b/test/transfer-manager.ts @@ -41,6 +41,7 @@ import fs from 'fs'; import {promises as fsp, Stats} from 'fs'; import * as sinon from 'sinon'; +import {FileExceptionMessages, RequestError} from '../src/file.js'; describe('Transfer Manager', () => { const BUCKET_NAME = 'test-bucket'; @@ -218,7 +219,10 @@ describe('Transfer Manager', () => { it('sets the destination correctly when provided a prefix', async () => { const prefix = 'test-prefix'; const filename = 'first.txt'; - const expectedDestination = path.normalize(`${prefix}/${filename}`); + const expectedDestination = path.resolve( + process.cwd(), + path.join(prefix, filename) + ); const file = new File(bucket, filename); sandbox.stub(file, 'download').callsFake(options => { @@ -233,7 +237,7 @@ describe('Transfer Manager', () => { it('sets the destination correctly when provided a strip prefix', async () => { const stripPrefix = 'should-be-removed/'; const filename = 'should-be-removed/first.txt'; - const expectedDestination = 'first.txt'; + const expectedDestination = path.resolve(process.cwd(), 'first.txt'); const file = new File(bucket, filename); sandbox.stub(file, 'download').callsFake(options => { @@ -263,8 +267,10 @@ describe('Transfer Manager', () => { destination: 'test-destination', }; const filename = 'first.txt'; - const expectedDestination = path.normalize( - `${passthroughOptions.destination}/${filename}` + const expectedDestination = path.resolve( + process.cwd(), + passthroughOptions.destination, + filename ); const download = (optionsOrCb?: DownloadOptions | DownloadCallback) => { if (typeof optionsOrCb === 'function') { @@ -280,6 +286,57 @@ describe('Transfer Manager', () => { await transferManager.downloadManyFiles([file], {passthroughOptions}); }); + it('should throws an error for absolute file names', async () => { + const expectedErr = new RequestError( + FileExceptionMessages.ABSOLUTE_FILE_NAME + ); + const maliciousFilename = '/etc/passwd'; + const file = new File(bucket, maliciousFilename); + + await assert.rejects( + transferManager.downloadManyFiles([file]), + expectedErr + ); + }); + + it('should throw an error for path traversal in destination', async () => { + const expectedErr = new RequestError( + FileExceptionMessages.TRAVERSAL_OUTSIDE_BASE_DESTINATION + ); + const passthroughOptions = { + destination: '../traversal-destination', + }; + const file = new File(bucket, 'first.txt'); + await assert.rejects( + transferManager.downloadManyFiles([file], {passthroughOptions}), + expectedErr + ); + }); + + it('should throw an error for path traversal in file name', async () => { + const expectedErr = new RequestError( + FileExceptionMessages.TRAVERSAL_OUTSIDE_BASE + ); + const file = new File(bucket, '../traversal-filename.txt'); + await assert.rejects( + transferManager.downloadManyFiles([file]), + expectedErr + ); + }); + + it('should throw an error for path traversal using prefix', async () => { + const expectedErr = new RequestError( + FileExceptionMessages.TRAVERSAL_OUTSIDE_BASE + ); + const file = new File(bucket, 'first.txt'); + await assert.rejects( + transferManager.downloadManyFiles([file], { + prefix: '../traversal-prefix', + }), + expectedErr + ); + }); + it('does not download files that already exist locally when skipIfExists is true', async () => { const firstFile = new File(bucket, 'first.txt'); sandbox.stub(firstFile, 'download').callsFake(options => { @@ -301,14 +358,16 @@ describe('Transfer Manager', () => { await transferManager.downloadManyFiles(files, options); }); - it('does not set the destination when prefix, strip prefix and passthroughOptions.destination are not provided', async () => { + it('sets the destination to CWD when prefix, strip prefix and passthroughOptions.destination are not provided', async () => { const options = {}; const filename = 'first.txt'; + const expectedDestination = path.resolve(process.cwd(), filename); + const download = (optionsOrCb?: DownloadOptions | DownloadCallback) => { if (typeof optionsOrCb === 'function') { optionsOrCb(null, Buffer.alloc(0)); } else if (optionsOrCb) { - assert.strictEqual(optionsOrCb.destination, undefined); + assert.strictEqual(optionsOrCb.destination, expectedDestination); } return Promise.resolve([Buffer.alloc(0)]) as Promise; }; @@ -321,10 +380,16 @@ describe('Transfer Manager', () => { it('should recursively create directory and write file contents if destination path is nested', async () => { const prefix = 'text-prefix'; const folder = 'nestedFolder/'; - const file = 'first.txt'; - const filesOrFolder = [folder, path.join(folder, file)]; - const expectedFilePath = path.join(prefix, folder, file); - const expectedDir = path.join(prefix, folder); + const filename = 'first.txt'; + const filesOrFolder = [folder, path.join(folder, filename)]; + const dirNameWithPrefix = path.join(prefix, folder); + const normalizedDir = path.resolve(process.cwd(), dirNameWithPrefix); + const expectedDir = normalizedDir + path.sep; + const expectedFilePath = path.resolve( + process.cwd(), + path.join(prefix, folder, filename) + ); + const mkdirSpy = sandbox.spy(fsp, 'mkdir'); const download = (optionsOrCb?: DownloadOptions | DownloadCallback) => { if (typeof optionsOrCb === 'function') { @@ -335,16 +400,21 @@ describe('Transfer Manager', () => { return Promise.resolve([Buffer.alloc(0)]) as Promise; }; - sandbox.stub(bucket, 'file').callsFake(filename => { - const file = new File(bucket, filename); - file.download = download; + sandbox.stub(bucket, 'file').callsFake(objectName => { + const file = new File(bucket, objectName); + if (objectName === path.join(folder, filename)) { + file.download = download; + } else { + file.download = () => + Promise.resolve([Buffer.alloc(0)]) as Promise; + } return file; }); await transferManager.downloadManyFiles(filesOrFolder, { prefix: prefix, }); assert.strictEqual( - mkdirSpy.calledOnceWith(expectedDir, { + mkdirSpy.calledWith(expectedDir, { recursive: true, }), true From 005ade2ac150d49de7c6b1ec00ba07403f087089 Mon Sep 17 00:00:00 2001 From: "gcf-owl-bot[bot]" <78513119+gcf-owl-bot[bot]@users.noreply.github.com> Date: Mon, 6 Oct 2025 10:05:21 -0400 Subject: [PATCH 6/7] chore: disable renovate for Node github action YAML configs (#2658) chore: disable renovate for github action YAML configs Source-Link: https://github.com/googleapis/synthtool/commit/158d49d854395e4eca4706df556628c418037193 Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-nodejs:latest@sha256:bdf89cdfb5b791d382184a7a769862b15c38e94e7d82b268c58d40d8952720f2 Co-authored-by: Owl Bot --- .github/.OwlBot.lock.yaml | 5 +- .github/scripts/close-invalid-link.cjs | 41 +++++-- .github/scripts/close-unresponsive.cjs | 108 ++++++++--------- .github/scripts/fixtures/invalidIssueBody.txt | 50 ++++++++ .github/scripts/fixtures/validIssueBody.txt | 50 ++++++++ .../validIssueBodyDifferentLinkLocation.txt | 50 ++++++++ .github/scripts/remove-response-label.cjs | 28 ++--- .../scripts/tests/close-invalid-link.test.cjs | 86 ++++++++++++++ .../close-or-remove-response-label.test.cjs | 109 ++++++++++++++++++ .github/workflows/issues-no-repro.yaml | 7 +- .github/workflows/response.yaml | 4 +- README.md | 2 +- renovate.json | 4 + 13 files changed, 457 insertions(+), 87 deletions(-) create mode 100644 .github/scripts/fixtures/invalidIssueBody.txt create mode 100644 .github/scripts/fixtures/validIssueBody.txt create mode 100644 .github/scripts/fixtures/validIssueBodyDifferentLinkLocation.txt create mode 100644 .github/scripts/tests/close-invalid-link.test.cjs create mode 100644 .github/scripts/tests/close-or-remove-response-label.test.cjs diff --git a/.github/.OwlBot.lock.yaml b/.github/.OwlBot.lock.yaml index d4140f443..0bb507a61 100644 --- a/.github/.OwlBot.lock.yaml +++ b/.github/.OwlBot.lock.yaml @@ -13,6 +13,5 @@ # limitations under the License. docker: image: gcr.io/cloud-devrel-public-resources/owlbot-nodejs:latest - digest: sha256:d0befde9bb710526253d1badc2d5b02884b466acc99db4e26ce8e71e69072ea0 -# created: 2025-03-07T03:28:55.703836867Z - + digest: sha256:bdf89cdfb5b791d382184a7a769862b15c38e94e7d82b268c58d40d8952720f2 +# created: 2025-10-03T19:51:38.870830821Z diff --git a/.github/scripts/close-invalid-link.cjs b/.github/scripts/close-invalid-link.cjs index d7a3688e7..fdb514881 100644 --- a/.github/scripts/close-invalid-link.cjs +++ b/.github/scripts/close-invalid-link.cjs @@ -12,21 +12,26 @@ // See the License for the specific language governing permissions and // limitations under the License. +const fs = require('fs'); +const yaml = require('js-yaml'); +const path = require('path'); +const TEMPLATE_FILE_PATH = path.resolve(__dirname, '../ISSUE_TEMPLATE/bug_report.yml') + async function closeIssue(github, owner, repo, number) { await github.rest.issues.createComment({ owner: owner, repo: repo, issue_number: number, - body: 'Issue was opened with an invalid reproduction link. Please make sure the repository is a valid, publicly-accessible github repository, and make sure the url is complete (example: https://github.com/googleapis/google-cloud-node)' + body: "Issue was opened with an invalid reproduction link. Please make sure the repository is a valid, publicly-accessible github repository, and make sure the url is complete (example: https://github.com/googleapis/google-cloud-node)" }); await github.rest.issues.update({ owner: owner, repo: repo, issue_number: number, - state: 'closed' + state: "closed" }); } -module.exports = async ({github, context}) => { +module.exports = async ({ github, context }) => { const owner = context.repo.owner; const repo = context.repo.repo; const number = context.issue.number; @@ -37,20 +42,32 @@ module.exports = async ({github, context}) => { issue_number: number, }); - const isBugTemplate = issue.data.body.includes('Link to the code that reproduces this issue'); + const yamlData = fs.readFileSync(TEMPLATE_FILE_PATH, 'utf8'); + const obj = yaml.load(yamlData); + const linkMatchingText = (obj.body.find(x => {return x.type === 'input' && x.validations.required === true && x.attributes.label.includes('link')})).attributes.label; + const isBugTemplate = issue.data.body.includes(linkMatchingText); if (isBugTemplate) { console.log(`Issue ${number} is a bug template`) try { - const link = issue.data.body.split('\n')[18].match(/(https?:\/\/(gist\.)?github.com\/.*)/)[0]; - console.log(`Issue ${number} contains this link: ${link}`) - const isValidLink = (await fetch(link)).ok; - console.log(`Issue ${number} has a ${isValidLink ? 'valid' : 'invalid'} link`) - if (!isValidLink) { - await closeIssue(github, owner, repo, number); - } + const text = issue.data.body; + const match = text.indexOf(linkMatchingText); + if (match !== -1) { + const nextLineIndex = text.indexOf('http', match); + if (nextLineIndex == -1) { + await closeIssue(github, owner, repo, number); + return; + } + const link = text.substring(nextLineIndex, text.indexOf('\n', nextLineIndex)); + console.log(`Issue ${number} contains this link: ${link}`); + const isValidLink = (await fetch(link)).ok; + console.log(`Issue ${number} has a ${isValidLink ? "valid" : "invalid"} link`) + if (!isValidLink) { + await closeIssue(github, owner, repo, number); + } + } } catch (err) { await closeIssue(github, owner, repo, number); } } -}; +}; \ No newline at end of file diff --git a/.github/scripts/close-unresponsive.cjs b/.github/scripts/close-unresponsive.cjs index 142dc1265..6f81b508f 100644 --- a/.github/scripts/close-unresponsive.cjs +++ b/.github/scripts/close-unresponsive.cjs @@ -1,4 +1,4 @@ -// Copyright 2024 Google LLC +/// Copyright 2024 Google LLC // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -13,57 +13,57 @@ // limitations under the License. function labeledEvent(data) { - return data.event === 'labeled' && data.label.name === 'needs more info'; - } - - const numberOfDaysLimit = 15; - const close_message = `This has been closed since a request for information has \ - not been answered for ${numberOfDaysLimit} days. It can be reopened when the \ - requested information is provided.`; - - module.exports = async ({github, context}) => { - const owner = context.repo.owner; - const repo = context.repo.repo; - - const issues = await github.rest.issues.listForRepo({ - owner: owner, - repo: repo, - labels: 'needs more info', - }); - const numbers = issues.data.map((e) => e.number); - - for (const number of numbers) { - const events = await github.paginate( - github.rest.issues.listEventsForTimeline, - { - owner: owner, - repo: repo, - issue_number: number, - }, - (response) => response.data.filter(labeledEvent) - ); - - const latest_response_label = events[events.length - 1]; - - const created_at = new Date(latest_response_label.created_at); - const now = new Date(); - const diff = now - created_at; - const diffDays = diff / (1000 * 60 * 60 * 24); - - if (diffDays > numberOfDaysLimit) { - await github.rest.issues.update({ - owner: owner, - repo: repo, - issue_number: number, - state: 'closed', - }); - - await github.rest.issues.createComment({ - owner: owner, - repo: repo, - issue_number: number, - body: close_message, - }); - } + return data.event === "labeled" && data.label.name === "needs more info"; +} + +const numberOfDaysLimit = 15; +const close_message = `This has been closed since a request for information has \ +not been answered for ${numberOfDaysLimit} days. It can be reopened when the \ +requested information is provided.`; + +module.exports = async ({ github, context }) => { + const owner = context.repo.owner; + const repo = context.repo.repo; + + const issues = await github.rest.issues.listForRepo({ + owner: owner, + repo: repo, + labels: "needs more info", + }); + const numbers = issues.data.map((e) => e.number); + + for (const number of numbers) { + const events = await github.paginate( + github.rest.issues.listEventsForTimeline, + { + owner: owner, + repo: repo, + issue_number: number, + }, + (response) => response.data.filter(labeledEvent) + ); + + const latest_response_label = events[events.length - 1]; + + const created_at = new Date(latest_response_label.created_at); + const now = new Date(); + const diff = now - created_at; + const diffDays = diff / (1000 * 60 * 60 * 24); + + if (diffDays > numberOfDaysLimit) { + await github.rest.issues.update({ + owner: owner, + repo: repo, + issue_number: number, + state: "closed", + }); + + await github.rest.issues.createComment({ + owner: owner, + repo: repo, + issue_number: number, + body: close_message, + }); } - }; + } +}; \ No newline at end of file diff --git a/.github/scripts/fixtures/invalidIssueBody.txt b/.github/scripts/fixtures/invalidIssueBody.txt new file mode 100644 index 000000000..504bd6690 --- /dev/null +++ b/.github/scripts/fixtures/invalidIssueBody.txt @@ -0,0 +1,50 @@ +### Please make sure you have searched for information in the following guides. + +- [X] Search the issues already opened: https://github.com/GoogleCloudPlatform/google-cloud-node/issues +- [X] Search StackOverflow: http://stackoverflow.com/questions/tagged/google-cloud-platform+node.js +- [X] Check our Troubleshooting guide: https://googlecloudplatform.github.io/google-cloud-node/#/docs/guides/troubleshooting +- [X] Check our FAQ: https://googlecloudplatform.github.io/google-cloud-node/#/docs/guides/faq +- [X] Check our libraries HOW-TO: https://github.com/googleapis/gax-nodejs/blob/main/client-libraries.md +- [X] Check out our authentication guide: https://github.com/googleapis/google-auth-library-nodejs +- [X] Check out handwritten samples for many of our APIs: https://github.com/GoogleCloudPlatform/nodejs-docs-samples + +### A screenshot that you have tested with "Try this API". + + +N/A + +### Link to the code that reproduces this issue. A link to a **public** Github Repository or gist with a minimal reproduction. + +not-a-link + +### A step-by-step description of how to reproduce the issue, based on the linked reproduction. + + +Change MY_PROJECT to your project name, add credentials if needed and run. + +### A clear and concise description of what the bug is, and what you expected to happen. + +The application crashes with the following exception (which there is no way to catch). It should just emit error, and allow graceful handling. +TypeError [ERR_INVALID_ARG_TYPE]: The "chunk" argument must be of type string or an instance of Buffer or Uint8Array. Received an instance of Object + at _write (node:internal/streams/writable:474:13) + at Writable.write (node:internal/streams/writable:502:10) + at Duplexify._write (/project/node_modules/duplexify/index.js:212:22) + at doWrite (/project/node_modules/duplexify/node_modules/readable-stream/lib/_stream_writable.js:390:139) + at writeOrBuffer (/project/node_modules/duplexify/node_modules/readable-stream/lib/_stream_writable.js:381:5) + at Writable.write (/project/node_modules/duplexify/node_modules/readable-stream/lib/_stream_writable.js:302:11) + at Pumpify. (/project/node_modules/@google-cloud/speech/build/src/helpers.js:79:27) + at Object.onceWrapper (node:events:633:26) + at Pumpify.emit (node:events:518:28) + at obj. [as _write] (/project/node_modules/stubs/index.js:28:22) + at doWrite (/project/node_modules/duplexify/node_modules/readable-stream/lib/_stream_writable.js:390:139) + at writeOrBuffer (/project/node_modules/duplexify/node_modules/readable-stream/lib/_stream_writable.js:381:5) + at Writable.write (/project/node_modules/duplexify/node_modules/readable-stream/lib/_stream_writable.js:302:11) + at PassThrough.ondata (node:internal/streams/readable:1007:22) + at PassThrough.emit (node:events:518:28) + at addChunk (node:internal/streams/readable:559:12) { + code: 'ERR_INVALID_ARG_TYPE' + + +### A clear and concise description WHY you expect this behavior, i.e., was it a recent change, there is documentation that points to this behavior, etc. ** + +No library should crash an application this way. \ No newline at end of file diff --git a/.github/scripts/fixtures/validIssueBody.txt b/.github/scripts/fixtures/validIssueBody.txt new file mode 100644 index 000000000..6e0ace338 --- /dev/null +++ b/.github/scripts/fixtures/validIssueBody.txt @@ -0,0 +1,50 @@ +### Please make sure you have searched for information in the following guides. + +- [X] Search the issues already opened: https://github.com/GoogleCloudPlatform/google-cloud-node/issues +- [X] Search StackOverflow: http://stackoverflow.com/questions/tagged/google-cloud-platform+node.js +- [X] Check our Troubleshooting guide: https://googlecloudplatform.github.io/google-cloud-node/#/docs/guides/troubleshooting +- [X] Check our FAQ: https://googlecloudplatform.github.io/google-cloud-node/#/docs/guides/faq +- [X] Check our libraries HOW-TO: https://github.com/googleapis/gax-nodejs/blob/main/client-libraries.md +- [X] Check out our authentication guide: https://github.com/googleapis/google-auth-library-nodejs +- [X] Check out handwritten samples for many of our APIs: https://github.com/GoogleCloudPlatform/nodejs-docs-samples + +### A screenshot that you have tested with "Try this API". + + +N/A + +### Link to the code that reproduces this issue. A link to a **public** Github Repository or gist with a minimal reproduction. + +https://gist.github.com/orgads/13cbf44c91923da27d8772b5f10489c9 + +### A step-by-step description of how to reproduce the issue, based on the linked reproduction. + + +Change MY_PROJECT to your project name, add credentials if needed and run. + +### A clear and concise description of what the bug is, and what you expected to happen. + +The application crashes with the following exception (which there is no way to catch). It should just emit error, and allow graceful handling. +TypeError [ERR_INVALID_ARG_TYPE]: The "chunk" argument must be of type string or an instance of Buffer or Uint8Array. Received an instance of Object + at _write (node:internal/streams/writable:474:13) + at Writable.write (node:internal/streams/writable:502:10) + at Duplexify._write (/project/node_modules/duplexify/index.js:212:22) + at doWrite (/project/node_modules/duplexify/node_modules/readable-stream/lib/_stream_writable.js:390:139) + at writeOrBuffer (/project/node_modules/duplexify/node_modules/readable-stream/lib/_stream_writable.js:381:5) + at Writable.write (/project/node_modules/duplexify/node_modules/readable-stream/lib/_stream_writable.js:302:11) + at Pumpify. (/project/node_modules/@google-cloud/speech/build/src/helpers.js:79:27) + at Object.onceWrapper (node:events:633:26) + at Pumpify.emit (node:events:518:28) + at obj. [as _write] (/project/node_modules/stubs/index.js:28:22) + at doWrite (/project/node_modules/duplexify/node_modules/readable-stream/lib/_stream_writable.js:390:139) + at writeOrBuffer (/project/node_modules/duplexify/node_modules/readable-stream/lib/_stream_writable.js:381:5) + at Writable.write (/project/node_modules/duplexify/node_modules/readable-stream/lib/_stream_writable.js:302:11) + at PassThrough.ondata (node:internal/streams/readable:1007:22) + at PassThrough.emit (node:events:518:28) + at addChunk (node:internal/streams/readable:559:12) { + code: 'ERR_INVALID_ARG_TYPE' + + +### A clear and concise description WHY you expect this behavior, i.e., was it a recent change, there is documentation that points to this behavior, etc. ** + +No library should crash an application this way. \ No newline at end of file diff --git a/.github/scripts/fixtures/validIssueBodyDifferentLinkLocation.txt b/.github/scripts/fixtures/validIssueBodyDifferentLinkLocation.txt new file mode 100644 index 000000000..984a420e3 --- /dev/null +++ b/.github/scripts/fixtures/validIssueBodyDifferentLinkLocation.txt @@ -0,0 +1,50 @@ +### Please make sure you have searched for information in the following guides. + +- [X] Search the issues already opened: https://github.com/GoogleCloudPlatform/google-cloud-node/issues +- [X] Search StackOverflow: http://stackoverflow.com/questions/tagged/google-cloud-platform+node.js +- [X] Check our Troubleshooting guide: https://googlecloudplatform.github.io/google-cloud-node/#/docs/guides/troubleshooting +- [X] Check our FAQ: https://googlecloudplatform.github.io/google-cloud-node/#/docs/guides/faq +- [X] Check our libraries HOW-TO: https://github.com/googleapis/gax-nodejs/blob/main/client-libraries.md +- [X] Check out our authentication guide: https://github.com/googleapis/google-auth-library-nodejs +- [X] Check out handwritten samples for many of our APIs: https://github.com/GoogleCloudPlatform/nodejs-docs-samples + +### A screenshot that you have tested with "Try this API". + + +N/A + +### A step-by-step description of how to reproduce the issue, based on the linked reproduction. + + +Change MY_PROJECT to your project name, add credentials if needed and run. + +### A clear and concise description of what the bug is, and what you expected to happen. + +The application crashes with the following exception (which there is no way to catch). It should just emit error, and allow graceful handling. +TypeError [ERR_INVALID_ARG_TYPE]: The "chunk" argument must be of type string or an instance of Buffer or Uint8Array. Received an instance of Object + at _write (node:internal/streams/writable:474:13) + at Writable.write (node:internal/streams/writable:502:10) + at Duplexify._write (/project/node_modules/duplexify/index.js:212:22) + at doWrite (/project/node_modules/duplexify/node_modules/readable-stream/lib/_stream_writable.js:390:139) + at writeOrBuffer (/project/node_modules/duplexify/node_modules/readable-stream/lib/_stream_writable.js:381:5) + at Writable.write (/project/node_modules/duplexify/node_modules/readable-stream/lib/_stream_writable.js:302:11) + at Pumpify. (/project/node_modules/@google-cloud/speech/build/src/helpers.js:79:27) + at Object.onceWrapper (node:events:633:26) + at Pumpify.emit (node:events:518:28) + at obj. [as _write] (/project/node_modules/stubs/index.js:28:22) + at doWrite (/project/node_modules/duplexify/node_modules/readable-stream/lib/_stream_writable.js:390:139) + at writeOrBuffer (/project/node_modules/duplexify/node_modules/readable-stream/lib/_stream_writable.js:381:5) + at Writable.write (/project/node_modules/duplexify/node_modules/readable-stream/lib/_stream_writable.js:302:11) + at PassThrough.ondata (node:internal/streams/readable:1007:22) + at PassThrough.emit (node:events:518:28) + at addChunk (node:internal/streams/readable:559:12) { + code: 'ERR_INVALID_ARG_TYPE' + +### Link to the code that reproduces this issue. A link to a **public** Github Repository with a minimal reproduction. + + +https://gist.github.com/orgads/13cbf44c91923da27d8772b5f10489c9 + +### A clear and concise description WHY you expect this behavior, i.e., was it a recent change, there is documentation that points to this behavior, etc. ** + +No library should crash an application this way. \ No newline at end of file diff --git a/.github/scripts/remove-response-label.cjs b/.github/scripts/remove-response-label.cjs index 887cf349e..4a784ddf7 100644 --- a/.github/scripts/remove-response-label.cjs +++ b/.github/scripts/remove-response-label.cjs @@ -13,21 +13,21 @@ // limitations under the License. module.exports = async ({ github, context }) => { - const commenter = context.actor; - const issue = await github.rest.issues.get({ + const commenter = context.actor; + const issue = await github.rest.issues.get({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: context.issue.number, + }); + const author = issue.data.user.login; + const labels = issue.data.labels.map((e) => e.name); + + if (author === commenter && labels.includes("needs more info")) { + await github.rest.issues.removeLabel({ owner: context.repo.owner, repo: context.repo.repo, issue_number: context.issue.number, + name: "needs more info", }); - const author = issue.data.user.login; - const labels = issue.data.labels.map((e) => e.name); - - if (author === commenter && labels.includes('needs more info')) { - await github.rest.issues.removeLabel({ - owner: context.repo.owner, - repo: context.repo.repo, - issue_number: context.issue.number, - name: 'needs more info', - }); - } - }; + } +}; \ No newline at end of file diff --git a/.github/scripts/tests/close-invalid-link.test.cjs b/.github/scripts/tests/close-invalid-link.test.cjs new file mode 100644 index 000000000..f63ee89c8 --- /dev/null +++ b/.github/scripts/tests/close-invalid-link.test.cjs @@ -0,0 +1,86 @@ +// Copyright 2024 Google LLC +// +// Licensed 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. + +'use strict'; + +const { describe, it } = require('mocha'); +const closeInvalidLink = require('../close-invalid-link.cjs'); +const fs = require('fs'); +const sinon = require('sinon'); + +describe('close issues with invalid links', () => { + let octokitStub; + let issuesStub; + + beforeEach(() => { + issuesStub = { + get: sinon.stub(), + createComment: sinon.stub(), + update: sinon.stub(), + }; + octokitStub = { + rest: { + issues: issuesStub, + }, + }; + }); + + afterEach(() => { + sinon.restore(); + }); + + it('does not do anything if it is not a bug', async () => { + const context = { repo: { owner: 'testOrg', repo: 'testRepo' }, issue: { number: 1 } }; + issuesStub.get.resolves({ data: { body: "I'm having a problem with this." } }); + + await closeInvalidLink({ github: octokitStub, context }); + + sinon.assert.calledOnce(issuesStub.get); + sinon.assert.notCalled(issuesStub.createComment); + sinon.assert.notCalled(issuesStub.update); + }); + + it('does not do anything if it is a bug with an appropriate link', async () => { + const context = { repo: { owner: 'testOrg', repo: 'testRepo' }, issue: { number: 1 } }; + issuesStub.get.resolves({ data: { body: fs.readFileSync('./fixtures/validIssueBody.txt', 'utf-8') } }); + + await closeInvalidLink({ github: octokitStub, context }); + + sinon.assert.calledOnce(issuesStub.get); + sinon.assert.notCalled(issuesStub.createComment); + sinon.assert.notCalled(issuesStub.update); + }); + + it('does not do anything if it is a bug with an appropriate link and the template changes', async () => { + const context = { repo: { owner: 'testOrg', repo: 'testRepo' }, issue: { number: 1 } }; + issuesStub.get.resolves({ data: { body: fs.readFileSync('./fixtures/validIssueBodyDifferentLinkLocation.txt', 'utf-8') } }); + + await closeInvalidLink({ github: octokitStub, context }); + + sinon.assert.calledOnce(issuesStub.get); + sinon.assert.notCalled(issuesStub.createComment); + sinon.assert.notCalled(issuesStub.update); + }); + + it('closes the issue if the link is invalid', async () => { + const context = { repo: { owner: 'testOrg', repo: 'testRepo' }, issue: { number: 1 } }; + issuesStub.get.resolves({ data: { body: fs.readFileSync('./fixtures/invalidIssueBody.txt', 'utf-8') } }); + + await closeInvalidLink({ github: octokitStub, context }); + + sinon.assert.calledOnce(issuesStub.get); + sinon.assert.calledOnce(issuesStub.createComment); + sinon.assert.calledOnce(issuesStub.update); + }); +}); \ No newline at end of file diff --git a/.github/scripts/tests/close-or-remove-response-label.test.cjs b/.github/scripts/tests/close-or-remove-response-label.test.cjs new file mode 100644 index 000000000..fb092c536 --- /dev/null +++ b/.github/scripts/tests/close-or-remove-response-label.test.cjs @@ -0,0 +1,109 @@ +// Copyright 2024 Google LLC +// +// Licensed 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. + +'use strict'; + +const { describe, it, beforeEach, afterEach } = require('mocha'); +const removeResponseLabel = require('../remove-response-label.cjs'); +const closeUnresponsive = require('../close-unresponsive.cjs'); +const sinon = require('sinon'); + +function getISODateDaysAgo(days) { + const today = new Date(); + const daysAgo = new Date(today.setDate(today.getDate() - days)); + return daysAgo.toISOString(); +} + +describe('close issues or remove needs more info labels', () => { + let octokitStub; + let issuesStub; + let paginateStub; + + beforeEach(() => { + issuesStub = { + listForRepo: sinon.stub(), + update: sinon.stub(), + createComment: sinon.stub(), + get: sinon.stub(), + removeLabel: sinon.stub(), + }; + paginateStub = sinon.stub(); + octokitStub = { + rest: { + issues: issuesStub, + }, + paginate: paginateStub, + }; + }); + + afterEach(() => { + sinon.restore(); + }); + + it('closes the issue if the OP has not responded within the allotted time and there is a needs-more-info label', async () => { + const context = { owner: 'testOrg', repo: 'testRepo' }; + const issuesInRepo = [{ user: { login: 'OP' }, labels: [{ name: 'needs more info' }] }]; + const eventsInIssue = [{ event: 'labeled', label: { name: 'needs more info' }, created_at: getISODateDaysAgo(16) }]; + + issuesStub.listForRepo.resolves({ data: issuesInRepo }); + paginateStub.resolves(eventsInIssue); + + await closeUnresponsive({ github: octokitStub, context }); + + sinon.assert.calledOnce(issuesStub.listForRepo); + sinon.assert.calledOnce(paginateStub); + sinon.assert.calledOnce(issuesStub.update); + sinon.assert.calledOnce(issuesStub.createComment); + }); + + it('does nothing if not enough time has passed and there is a needs-more-info label', async () => { + const context = { owner: 'testOrg', repo: 'testRepo' }; + const issuesInRepo = [{ user: { login: 'OP' }, labels: [{ name: 'needs more info' }] }]; + const eventsInIssue = [{ event: 'labeled', label: { name: 'needs more info' }, created_at: getISODateDaysAgo(14) }]; + + issuesStub.listForRepo.resolves({ data: issuesInRepo }); + paginateStub.resolves(eventsInIssue); + + await closeUnresponsive({ github: octokitStub, context }); + + sinon.assert.calledOnce(issuesStub.listForRepo); + sinon.assert.calledOnce(paginateStub); + sinon.assert.notCalled(issuesStub.update); + sinon.assert.notCalled(issuesStub.createComment); + }); + + it('removes the label if OP responded', async () => { + const context = { actor: 'OP', repo: { owner: 'testOrg', repo: 'testRepo' }, issue: { number: 1 } }; + const issueContext = { user: {login: 'OP'}, labels: [{ name: 'needs more info' }] }; + + issuesStub.get.resolves({ data: issueContext }); + + await removeResponseLabel({ github: octokitStub, context }); + + sinon.assert.calledOnce(issuesStub.get); + sinon.assert.calledOnce(issuesStub.removeLabel); + }); + + it('does not remove the label if author responded', async () => { + const context = { actor: 'repo-maintainer', repo: { owner: 'testOrg', repo: 'testRepo' }, issue: { number: 1 } }; + const issueContext = { user: {login: 'OP'}, labels: [{ name: 'needs more info' }] }; + + issuesStub.get.resolves({ data: issueContext }); + + await removeResponseLabel({ github: octokitStub, context }); + + sinon.assert.calledOnce(issuesStub.get); + sinon.assert.notCalled(issuesStub.removeLabel); + }); +}); \ No newline at end of file diff --git a/.github/workflows/issues-no-repro.yaml b/.github/workflows/issues-no-repro.yaml index 442a46bcc..531054022 100644 --- a/.github/workflows/issues-no-repro.yaml +++ b/.github/workflows/issues-no-repro.yaml @@ -10,7 +10,12 @@ jobs: issues: write pull-requests: write steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v5 + - uses: actions/setup-node@v4 + with: + node-version: 18 + - run: npm install + working-directory: ./.github/scripts - uses: actions/github-script@v7 with: script: | diff --git a/.github/workflows/response.yaml b/.github/workflows/response.yaml index 6ed37326f..e81a3603a 100644 --- a/.github/workflows/response.yaml +++ b/.github/workflows/response.yaml @@ -13,7 +13,7 @@ jobs: issues: write pull-requests: write steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v5 - uses: actions/github-script@v7 with: script: | @@ -27,7 +27,7 @@ jobs: issues: write pull-requests: write steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v5 - uses: actions/github-script@v7 with: script: | diff --git a/README.md b/README.md index 7e7979a40..81047c5e9 100644 --- a/README.md +++ b/README.md @@ -5,7 +5,7 @@ # [Google Cloud Storage: Node.js Client](https://github.com/googleapis/nodejs-storage) [![release level](https://img.shields.io/badge/release%20level-stable-brightgreen.svg?style=flat)](https://cloud.google.com/terms/launch-stages) -[![npm version](https://img.shields.io/npm/v/@google-cloud/storage.svg)](https://www.npmjs.org/package/@google-cloud/storage) +[![npm version](https://img.shields.io/npm/v/@google-cloud/storage.svg)](https://www.npmjs.com/package/@google-cloud/storage) diff --git a/renovate.json b/renovate.json index c5c702cf4..f39fd3232 100644 --- a/renovate.json +++ b/renovate.json @@ -15,6 +15,10 @@ { "extends": "packages:linters", "groupName": "linters" + }, + { + "matchManagers": ["github-actions"], + "enabled": false } ], "ignoreDeps": ["typescript"] From 70f706ed1f4be6127f862de1f8a1c16bf5d39b34 Mon Sep 17 00:00:00 2001 From: "release-please[bot]" <55107282+release-please[bot]@users.noreply.github.com> Date: Mon, 6 Oct 2025 10:24:13 -0400 Subject: [PATCH 7/7] chore(main): release 7.17.2 (#2653) Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com> --- CHANGELOG.md | 8 ++++++++ package.json | 2 +- samples/package.json | 2 +- 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 97e95c736..391562a0c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,14 @@ [1]: https://www.npmjs.com/package/@google-cloud/storage?activeTab=versions +## [7.17.2](https://github.com/googleapis/nodejs-storage/compare/v7.17.1...v7.17.2) (2025-10-06) + + +### Bug Fixes + +* Common Service: should retry a request failed ([#2652](https://github.com/googleapis/nodejs-storage/issues/2652)) ([b38b5d2](https://github.com/googleapis/nodejs-storage/commit/b38b5d221f2cb72658c1eb4a726315ab395a542c)) +* Implement path containment to prevent traversal attacks ([#2654](https://github.com/googleapis/nodejs-storage/issues/2654)) ([08d7abf](https://github.com/googleapis/nodejs-storage/commit/08d7abf32dd365b24ce34c66174be06c30bfce8f)) + ## [7.17.1](https://github.com/googleapis/nodejs-storage/compare/v7.17.0...v7.17.1) (2025-08-27) diff --git a/package.json b/package.json index 23434caf0..568f927a6 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "@google-cloud/storage", "description": "Cloud Storage Client Library for Node.js", - "version": "7.17.1", + "version": "7.17.2", "license": "Apache-2.0", "author": "Google Inc.", "engines": { diff --git a/samples/package.json b/samples/package.json index 6b1c10d2e..26266855f 100644 --- a/samples/package.json +++ b/samples/package.json @@ -17,7 +17,7 @@ }, "dependencies": { "@google-cloud/pubsub": "^4.0.0", - "@google-cloud/storage": "^7.17.1", + "@google-cloud/storage": "^7.17.2", "node-fetch": "^2.6.7", "uuid": "^8.0.0", "yargs": "^16.0.0"