You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@logging.apache.org by "ppkarwasz (via GitHub)" <gi...@apache.org> on 2025/03/10 09:28:24 UTC
[PR] Improve resilience to script injection [logging-parent]
ppkarwasz opened a new pull request, #344:
URL: https://github.com/apache/logging-parent/pull/344
This change removes all direct usages of GitHub expressions to prevent potential script injections.
**Note**: The GitHub expressions modified in this PR only come from **trusted** sources, so these problems are not exploitable. We wish, however, to prevent possible errors in the future.
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve resilience to script injection [logging-parent]
Posted by "vy (via GitHub)" <gi...@apache.org>.
vy commented on code in PR #344:
URL: https://github.com/apache/logging-parent/pull/344#discussion_r1986926284
##########
.github/workflows/deploy-release-reusable.yaml:
##########
@@ -55,6 +55,8 @@ on:
jobs:
deploy:
runs-on: ubuntu-latest
+ env:
+ PROJECT_ID: ${{ inputs.project-id }}
Review Comment:
I find the old style more secure: a global `input` injected wherever it is needed, instead of being exposed using a global environment variable to every script. My 2 cents...
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve resilience to script injection [logging-parent]
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on PR #344:
URL: https://github.com/apache/logging-parent/pull/344#issuecomment-2710079948
> @ppkarwasz, can you explain the hardening practiced by replacing `input.foo` statements with shell environment variables?
This follows the [Security Hardening for GitHub Actions](https://docs.github.com/en/actions/security-for-github-actions/security-guides/security-hardening-for-github-actions#good-practices-for-mitigating-script-injection-attacks) guide.
Let assume the `inputs.foo` contains untrusted data, for example `3.0.0"; echo Hello world!`. Then:
```
export version="${{ inputs.foo }}"
```
will execute the `echo` command.
As noted before the problem currently does not exist, since the inputs are either hard coded or come from trusted sources, but removing those potential problems is very cheap. I am sure that CodeQL will at some point mark issue warnings for those step definitions.
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve resilience to script injection [logging-parent]
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz merged PR #344:
URL: https://github.com/apache/logging-parent/pull/344
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve resilience to script injection [logging-parent]
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on code in PR #344:
URL: https://github.com/apache/logging-parent/pull/344#discussion_r2000815591
##########
.github/workflows/deploy-release-reusable.yaml:
##########
@@ -55,6 +55,8 @@ on:
jobs:
deploy:
runs-on: ubuntu-latest
+ env:
+ PROJECT_ID: ${{ inputs.project-id }}
Review Comment:
Fixed in https://github.com/apache/logging-parent/pull/344/commits/12583e2b04f773a290a53cc223b2c61794835faa
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve resilience to script injection [logging-parent]
Posted by "vy (via GitHub)" <gi...@apache.org>.
vy commented on PR #344:
URL: https://github.com/apache/logging-parent/pull/344#issuecomment-2709961573
@ppkarwasz, can you explain the hardening practiced by replacing `input.foo` statements with shell environment variables?
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org