Copilot commented on code in PR #8530:
URL: https://github.com/apache/hadoop/pull/8530#discussion_r3358356455
##########
.github/workflows/cloud_aws.yml:
##########
@@ -20,15 +20,100 @@
name: "Cloud-AWS"
on:
+ # pull requests will automatically trigger S3A tests for non-forked PRs
pull_request:
paths:
- 'hadoop-tools/hadoop-aws/**'
- '.github/workflows/*cloud_aws.yml'
- '.github/actions/build_image**'
- '.github/gha-tests/hadoop-aws*excludes.txt'
+ # For fork PRs, S3A integration tests must be manually triggered.
+ # Although our auth. key for localstack is not a very sensitive secret,
+ # we don't want to leak it to PRs that we haven't reviewed yet.
+ # A maintainer should confirm that affected fork PRs don't
+ # include questionable changes to .github/workflows/ (i.e. that may leak
+ # secrets) before approving the workflow run.
Review Comment:
The manual-trigger security note only mentions reviewing changes under
`.github/workflows/`, but this workflow run checks out and executes code from
the fork. That means changes under `.github/actions/` (and other scripts
invoked by the workflow) are also part of the trusted computing base and should
be explicitly called out here.
##########
.github/workflows/cloud_aws.yml:
##########
@@ -20,15 +20,100 @@
name: "Cloud-AWS"
on:
+ # pull requests will automatically trigger S3A tests for non-forked PRs
pull_request:
paths:
- 'hadoop-tools/hadoop-aws/**'
- '.github/workflows/*cloud_aws.yml'
- '.github/actions/build_image**'
- '.github/gha-tests/hadoop-aws*excludes.txt'
+ # For fork PRs, S3A integration tests must be manually triggered.
+ # Although our auth. key for localstack is not a very sensitive secret,
+ # we don't want to leak it to PRs that we haven't reviewed yet.
+ # A maintainer should confirm that affected fork PRs don't
+ # include questionable changes to .github/workflows/ (i.e. that may leak
+ # secrets) before approving the workflow run.
+ workflow_dispatch:
+ inputs:
+ pr_number:
+ description: Pull request number from a fork repository
+ required: true
+ type: string
+ java:
+ description: Java version for the Cloud-AWS run
+ required: false
+ type: string
+ default: '25'
+ os:
+ description: OS image key for the test container
+ required: false
+ type: string
+ default: ubuntu_24
+ runner_os:
+ description: Runner label used to execute jobs
+ required: false
+ type: string
+ default: ubuntu-24.04
+
jobs:
+ # Stash the owner, repo, and PR number, so we can properly add a test summary
+ # in both the forked-repo, and non-forked case.
+ resolve-fork-pr:
+ if: github.event_name == 'workflow_dispatch'
+ runs-on: ubuntu-slim
+ permissions:
+ actions: write
+ pull-requests: read
Review Comment:
`resolve-fork-pr` grants `actions: write`, but this job only reads PR
metadata and uploads an artifact. Dropping this to least privilege reduces the
blast radius of the manual workflow.
##########
.github/workflows/tmpl_cloud_aws.yml:
##########
@@ -63,8 +74,8 @@ jobs:
steps:
- uses: actions/checkout@v6
with:
- # Full fetch so build image URL can be computed for any branch
- fetch-depth: 0
+ repository: ${{ inputs.checkout_repository || github.repository }}
+ ref: ${{ inputs.checkout_ref || github.ref }}
Review Comment:
When running the manual fork PR workflow, `actions/checkout` will (by
default) persist the `GITHUB_TOKEN` credential into the git config of the
checked-out fork. Since subsequent steps execute untrusted code, set
`persist-credentials: false` to reduce the risk of token exfiltration/misuse.
##########
.github/workflows/cloud_aws.yml:
##########
@@ -20,15 +20,100 @@
name: "Cloud-AWS"
on:
+ # pull requests will automatically trigger S3A tests for non-forked PRs
pull_request:
paths:
- 'hadoop-tools/hadoop-aws/**'
- '.github/workflows/*cloud_aws.yml'
- '.github/actions/build_image**'
- '.github/gha-tests/hadoop-aws*excludes.txt'
+ # For fork PRs, S3A integration tests must be manually triggered.
+ # Although our auth. key for localstack is not a very sensitive secret,
+ # we don't want to leak it to PRs that we haven't reviewed yet.
+ # A maintainer should confirm that affected fork PRs don't
+ # include questionable changes to .github/workflows/ (i.e. that may leak
+ # secrets) before approving the workflow run.
+ workflow_dispatch:
+ inputs:
+ pr_number:
+ description: Pull request number from a fork repository
+ required: true
+ type: string
+ java:
+ description: Java version for the Cloud-AWS run
+ required: false
+ type: string
+ default: '25'
+ os:
+ description: OS image key for the test container
+ required: false
+ type: string
+ default: ubuntu_24
+ runner_os:
+ description: Runner label used to execute jobs
+ required: false
+ type: string
+ default: ubuntu-24.04
+
jobs:
+ # Stash the owner, repo, and PR number, so we can properly add a test summary
+ # in both the forked-repo, and non-forked case.
Review Comment:
The comment says this job stashes "owner, repo, and PR number", but the job
actually resolves and outputs the fork head repo/SHA (and writes those to an
artifact). Updating this avoids confusion when maintaining the workflow.
##########
.github/workflows/tmpl_cloud_aws.yml:
##########
@@ -87,6 +98,9 @@ jobs:
run: |
echo "Build image URL: ${{
needs.precondition.outputs.build_image_url }}"
- uses: actions/checkout@v6
+ with:
+ repository: ${{ inputs.checkout_repository || github.repository }}
+ ref: ${{ inputs.checkout_ref || github.ref }}
Review Comment:
Same token-persistence concern as the other checkout steps: this job checks
out the fork repo and then runs local composite actions and other steps.
Disabling credential persistence prevents leaving a usable `GITHUB_TOKEN` in
`.git/config` for untrusted code to read.
##########
.github/workflows/notify_cloud_aws.yml:
##########
@@ -0,0 +1,120 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF 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
+#
+# http://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.
+#
+
+# Add a sticky comment to hadoop-aws PRs from forked repos with a hint that
+# integration tests must be manually triggered by a maintainer.
+#
+name: "Cloud-AWS PR Update"
+
+# Security: This privileged workflow uses pull_request_target but does not
+# check out or execute untrusted code. It only creates a check run and a PR
+# comment in the base repository.
+on:
+ pull_request_target:
+ types: [opened, reopened, synchronize]
+ paths:
+ - 'hadoop-tools/hadoop-aws/**'
+ - '.github/workflows/*cloud_aws.yml'
+ - '.github/actions/build_image**'
+ - '.github/gha-tests/hadoop-aws*excludes.txt'
+
+jobs:
+ notify:
+ if: github.event.pull_request.head.repo.full_name != github.repository
+ name: "Notify Cloud-AWS"
+ runs-on: ubuntu-slim
+ permissions:
+ checks: write
+ pull-requests: write
+ steps:
+ - name: Post approval-required check and sticky comment
+ uses: actions/github-script@v9
+ with:
+ github-token: ${{ secrets.GITHUB_TOKEN }}
+ script: |
+ const marker = '<!-- cloud-aws-fork-pr-note -->';
+ const pr = context.payload.pull_request;
+ const workflowUrl =
`https://github.com/${context.repo.owner}/${context.repo.repo}/actions/workflows/cloud_aws.yml`;
+ const dispatchCommand = `gh workflow run cloud_aws.yml -R
${context.repo.owner}/${context.repo.repo} -f pr_number=${pr.number}`;
+
+ const checkTitle = 'Cloud-AWS manual trigger instructions';
+ const checkSummary = [
+ 'S3A tests must be manually triggered for fork pull requests.',
+ 'A maintainer should:\n',
+ '- Confirm the PR does not contain questionable changes to
actions in',
+ ' .github/workflows.\n',
+ '- *Approve workflows* for this fork PR if prompted.\n',
Review Comment:
The instructions in the sticky comment only tell maintainers to review
`.github/workflows`, but the manually-triggered run checks out and executes
code from the fork. Changes under `.github/actions/` should be reviewed as well
(they can alter behavior of steps like `uses: ./.github/actions/...`).
##########
.github/workflows/tmpl_cloud_aws.yml:
##########
@@ -141,6 +155,9 @@ jobs:
-Dcheckstyle.skip -Dspotbugs.skip -Denforcer.skip -Drat.skip
steps:
- uses: actions/checkout@v6
+ with:
+ repository: ${{ inputs.checkout_repository || github.repository }}
+ ref: ${{ inputs.checkout_ref || github.ref }}
Review Comment:
Same token-persistence concern as the other checkout steps: the test job
executes untrusted build/test code after checkout. Set `persist-credentials:
false` so the `GITHUB_TOKEN` isn't stored in the git config within the
workspace.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
