-
Notifications
You must be signed in to change notification settings - Fork 4.3k
fix(lambda-nodejs): use official bun installer for ARM64 compatibility #35535
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(lambda-nodejs): use official bun installer for ARM64 compatibility #35535
Conversation
…ility Replace npm-based bun installation with official installer to fix Docker bundling failures on ARM64 architectures (AWS Graviton). The npm package @oven/bun-linux-x64-baseline doesn't exist for ARM64, causing builds to fail on Graviton instances. The official bun installer automatically detects and downloads the correct platform binary. Fixes aws#35534 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The pull request linter fails with the following errors:
❌ Fixes must contain a change to a test file.
❌ Fixes must contain a change to an integration test file and the resulting snapshot.
If you believe this pull request should receive an exemption, please comment and provide a justification. A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed, add Clarification Request to a comment.
|
I actually don't want to change the tests at all. Ideally the tests should all run exactly as they are and should pass. We shouldn't really care what the source of the bun package is, but as long as it's installed then our test suite should prove that bun still works. Riiiiight? :) |
| # CDK's original: RUN npm install --global bun@1.1.30 | ||
| # Issue: @oven/bun-linux-x64-baseline npm package doesn't exist for ARM64 | ||
| # Solution: Use official installer that downloads correct platform binary | ||
| RUN curl -fsSL https://bun.sh/install | bash -s "bun-v1.1.30" && \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider Security Hardening like this as the external dependency (bun.sh) might be a security concern?
# Alternative with checksum verification (future enhancement)
RUN curl -fsSL https://bun.sh/install -o /tmp/install.sh && \
echo "expected_checksum /tmp/install.sh" | sha256sum -c && \
bash /tmp/install.sh "bun-v1.1.30" && \
ln -s /root/.bun/bin/bun /usr/local/bin/bun && \
rm /tmp/install.shThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @pahud, this is a good call. I'm DEFINITELY concerned about the security implications of this change and the risk of supply chain poisoning. Let me try this out and update the PR if it all works
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ug. The trouble here is that your suggestion (which is good) verifies the installer hasn't changed. We likely want to do that... but we also likely want to confirm the sha hash on the downloaded binary... which is significantly harder.
The hashes are available on github (yay) but the installer doesn't have a way to check them during installation or confirm them before install. And in looking over bun's installer docs I don't see anything that shows a way to do installation of the package without the installer. So downloading the correct binary, validating it, and installing it, is challenging. Though likely a good idea.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure what to do here. A breaking change was introduced in a feature that we don't need (we don't even use bun). It wasn't well tested and CDK does not do semver, so there was no warning.
My instinct here is to rip bun out or make it optional in some way as there isn't a secure way to solve this problem, and I don't need the feature. But that's selfish and not helpful to people who DO need this feature.
I defer to you for guidance @pahud. What do you think is a correct forward path? I want to make sure we're being secure here, especially given the potential blast radius of a compromise.
I think the most robust solution would be to pull the package we need from github, validate the sha against a known good sha, and then install it. But that installation will require additional manual steps and perhaps recreation of the bun installer script. Yuck.
|
I'm Closing this PR in favor of one which bumps the package version of Bun to the latest. It appears there is a proper version released for the newer version. Assuming that bun follows proper semver, then there should be no reason to think that this upgrade won't behave as expected. |
|
Comments on closed issues and PRs are hard for our team to see. |
Description
Fixes Docker bundling failures when using bun on ARM64 architectures (AWS Graviton instances).
Problem
The current Dockerfile uses
npm install --global bun@1.1.30which fails on ARM64 because the npm package@oven/bun-linux-x64-baselinedoesn't exist for ARM64 architecture.Solution
Replace npm-based installation with the official bun installer that automatically detects and downloads the correct platform binary.
Changes
RUN npm install --global bun@1.1.30RUN curl -fsSL https://bun.sh/install | bash -s "bun-v1.1.30" && ln -s /root/.bun/bin/bun /usr/local/bin/bunSecurity Considerations
Risk Mitigation Options (for future consideration):
Testing
Closes #35534
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license