dev.Dockerfile: add BUILD_TAGS build argument#1244
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
- `dev.Dockerfile` now has a `BUILD_TAGS` build argument that allows the `tags` passed to `make` to be customized. If defined, the defaults are completely overriden (not added to). If not defined, the default values are used.
5e0e23a to
cbedfd8
Compare
|
force pushed with the actual PR number linked in the release notes. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a BUILD_TAGS argument to dev.Dockerfile for customizing build tags during go install commands. However, the current implementation is vulnerable to command injection as BUILD_TAGS is directly interpolated into shell commands within RUN instructions, posing a risk of arbitrary command execution. Furthermore, the Makefile does not correctly consume this tags variable for go install -tags or build.RawTags in LDFLAGS, leading to a functional discrepancy where the BUILD_TAGS argument will not effectively override or augment build tags as intended. It is crucial to address the command injection vulnerability by sanitizing input or using a safer argument passing method, and to update the Makefile to properly utilize BUILD_TAGS.
| make go-install-noui tags="$BUILD_TAGS"\ | ||
| && make go-install-cli-noui tags="$BUILD_TAGS"; \ | ||
| else \ | ||
| make go-install \ | ||
| && make go-install-cli; \ | ||
| make go-install tags="$BUILD_TAGS" \ | ||
| && make go-install-cli tags="$BUILD_TAGS"; \ |
There was a problem hiding this comment.
This section is vulnerable to command injection. The BUILD_TAGS argument is directly interpolated into a shell command within a RUN instruction. An attacker controlling BUILD_TAGS could inject arbitrary commands (e.g., foo" ; rm -rf / ; "bar). Additionally, the Makefile targets (go-install-noui, go-install) explicitly use $(LND_RELEASE_TAGS) instead of the tags variable passed from the Dockerfile for go install -tags and build.RawTags in LDFLAGS. This means the BUILD_TAGS value is not being used as intended to customize build tags. To fix this, sanitize the BUILD_TAGS input to prevent injection and update the Makefile targets to conditionally use the tags variable for both go install -tags and build.RawTags.
There was a problem hiding this comment.
Additionally, the
Makefiletargets (go-install-noui,go-install) explicitly use$(LND_RELEASE_TAGS)instead of thetagsvariable passed from the Dockerfile forgo install -tagsandbuild.RawTagsinLDFLAGS. This means theBUILD_TAGSvalue is not being used as intended to customize build tags.
Isn't
lightning-terminal/make/release_flags.mk
Lines 35 to 38 in 6f12830
There was a problem hiding this comment.
This section is vulnerable to command injection. The
BUILD_TAGSargument is directly interpolated into a shell command within aRUNinstruction. An attacker controllingBUILD_TAGScould inject arbitrary commands (e.g.,foo" ; rm -rf / ; "bar).
Are we worried about this on dev builds where we control the machine being built on?
There are no reviewers that have been assigned that I can re-request review from. |
|
@ZZiigguurraatt, remember to re-request review from reviewers when ready |
2 participants
dev.Dockerfilenow has aBUILD_TAGSbuild argument that allows thetagspassed tomaketo be customized. If defined, the defaults are completely overriden (not added to). If not defined, the default values are used.