KEMBAR78
chore: secure hermetic_library_generation workflow by diegomarquezp · Pull Request #2317 · googleapis/java-bigtable · GitHub
Skip to content

Conversation

@diegomarquezp
Copy link
Contributor

Thanks to @diogoteles08 for the inspection on our repos. This PR inlines environment variables to avoid overriding script injections.

Thanks to @diogoteles08 for the inspection on our repos.
This PR inlines environment variables to avoid overriding script injections.
@diegomarquezp diegomarquezp requested review from a team as code owners August 19, 2024 20:19
@product-auto-label product-auto-label bot added size: xs Pull request size is extra small. api: bigtable Issues related to the googleapis/java-bigtable API. labels Aug 19, 2024
@product-auto-label product-auto-label bot added size: s Pull request size is small. and removed size: xs Pull request size is extra small. labels Aug 22, 2024
outputs:
REPO_FULL_NAME: ${{ env.REPO_FULL_NAME }}
steps:
- run: echo "" >> /dev/null # no op - we just need to declare the env var as output
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this empty step required?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, yes
image

However, we can make it even simpler (exit 0)

# skip pull requests come from a forked repository
if: github.event.pull_request.head.repo.full_name == github.repository
# skip pull requests coming from a forked repository
if: needs.prepare-repo-full-name.outputs.REPO_FULL_NAME == github.repository
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if we move this if condition to the run section? Did we consider the approach before @JoeWang1127 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I modified it to just have an if inside run. I'll try a fork PR into this branch

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried raising a PR from a fork
image

I tried setting the missing token in my fork without luck.
However I could confirm that the generation was properly triggered in this PR because it doesn't come from a fork.
https://github.com/googleapis/java-bigtable/actions/runs/10562926017/job/29261969452?pr=2317

@JoeWang1127
Copy link
Contributor

Could you create a forked repo and test whether the workflow will be skipped?

@diegomarquezp
Copy link
Contributor Author

Could you create a forked repo and test whether the workflow will be skipped?

@JoeWang1127 we tried this in #2317 (comment). I think we still confirmed it works the other way around: non-forked repos actually get triggered.

@diegomarquezp diegomarquezp merged commit 1e9c8ab into main Sep 3, 2024
@diegomarquezp diegomarquezp deleted the diegomarquezp-patch-1 branch September 3, 2024 15:16
JoeWang1127 added a commit to googleapis/sdk-platform-java that referenced this pull request Sep 4, 2024
…ains common protos (#3162)

In this PR:
- Always perform a full generation in a non-monorepo or the repo
contains common protos.
- Secure generation workflow (use environment variable to avoid script
injection), inspired by
googleapis/java-bigtable#2317

This PR also brings changes in common protos and iam due to protoc
updates (25.3 -> 25.4).

---------

Co-authored-by: cloud-java-bot <cloud-java-bot@google.com>
ldetmer pushed a commit to googleapis/sdk-platform-java that referenced this pull request Sep 17, 2024
…ains common protos (#3162)

In this PR:
- Always perform a full generation in a non-monorepo or the repo
contains common protos.
- Secure generation workflow (use environment variable to avoid script
injection), inspired by
googleapis/java-bigtable#2317

This PR also brings changes in common protos and iam due to protoc
updates (25.3 -> 25.4).

---------

Co-authored-by: cloud-java-bot <cloud-java-bot@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: bigtable Issues related to the googleapis/java-bigtable API. size: s Pull request size is small.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants