-
Notifications
You must be signed in to change notification settings - Fork 153
Add build auth file injection for protocol builds #2909
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
base: main
Are you sure you want to change the base?
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2909 +/- ##
==========================================
- Coverage 56.36% 56.25% -0.11%
==========================================
Files 323 324 +1
Lines 31764 31898 +134
==========================================
+ Hits 17904 17945 +41
- Misses 12331 12423 +92
- Partials 1529 1530 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
Large PR Detected
This PR exceeds 1000 lines of changes and requires justification before it can be reviewed.
How to unblock this PR:
Add a section to your PR description with the following format:
## Large PR Justification
[Explain why this PR must be large, such as:]
- Generated code that cannot be split
- Large refactoring that must be atomic
- Multiple related changes that would break if separated
- Migration or data transformationAlternative:
Consider splitting this PR into smaller, focused changes (< 1000 lines each) for easier review and reduced risk.
See our Contributing Guidelines for more details.
This review will be automatically dismissed once you add the justification section.
Large PR justification has been provided. Thank you!
|
✅ Large PR justification has been provided. The size review has been dismissed and this PR can now proceed with normal review. |
jhrozek
left a comment
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 will admit that this was an LLM finding but it seems important. Otherwise the pR looks good to me.
Add support for injecting authentication files (npmrc, netrc, yarnrc) into container builds for protocol schemes (npx://, uvx://, go://). This solves the problem that NPM registry-scoped authentication cannot be configured via environment variables because the format (//registry.example.com/:_authToken=TOKEN) contains characters that are invalid as environment variable names. Key features: - New CLI commands: set-build-auth-file, get-build-auth-file, unset-build-auth-file - Files are injected into the builder stage only and NOT included in the final container image for security - Supports npmrc (npm/npx), netrc (pip/Go), and yarnrc (Yarn) - Credentials are hidden by default in CLI output (use --show-content to display) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
This avoids exposing secrets in shell history and process listings. Examples: cat ~/.npmrc | thv config set-build-auth-file npmrc --stdin thv config set-build-auth-file npmrc --stdin < ~/.npmrc 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Refactor build auth file storage to use ToolHive's encrypted secrets provider instead of storing credentials in plain text config files. - Config now stores only markers (e.g., "secret:BUILD_AUTH_FILE_npmrc") - Actual content stored in keyring/encrypted secrets provider - CLI commands updated to use secrets manager for storage/retrieval - protocol.go resolves secrets at build time - Updated interface methods and all provider implementations This ensures sensitive credentials like NPM tokens are never stored in plain text on disk. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
4e5ebb5 to
2e24d46
Compare
…image When building from a local path (e.g., go://./myapp), the build context is copied to /build/ which may include auth files like .netrc, .npmrc, or .yarnrc. The final stage then copies /build/ to /app/, which would leak these credentials to the final container image. Add cleanup steps after the build completes (so auth was used during package fetching) but before the final stage copies from /build/: - go.tmpl: RUN rm -f /build/.netrc /build/.npmrc /build/.yarnrc /root/.netrc - npx.tmpl: RUN rm -f /build/.netrc /build/.npmrc /build/.yarnrc /root/.npmrc - uvx.tmpl: RUN rm -f /build/.netrc /build/.npmrc /build/.yarnrc /root/.netrc This only affects local path builds (IsLocalPath=true). Remote package builds (e.g., npx://@org/package) are unaffected since they only copy specific files like node_modules or package.json. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
|
Thanks for catching this @jhrozek! Good find - I've added cleanup steps to all three templates (go, npx, uvx) in commit 7f9ddf7. The issue only affects local path builds (e.g., The fix adds |
|
Hey, quick security issue or more like concern about the fix, removing files with because docker layers are immutable if I am not wrong, so when you do so you build with docker build --secret id=npmrc,src=.npmrc .so its secret is never ever stored and never stored in any layer, so its temprorarily during the RUN command |
|
Yeah, this is mostly an issue for local path builds. Protocol builds with packages (the most common scenario) use multi-stage builds, and the secret is only persisted on the initial and not the end container image. |
Summary
//registry.example.com/:_authToken=TOKEN) contains characters that are invalid as environment variable namesLarge PR Justification
This PR is 1000+ lines but the changes are cohesive and cannot be easily split:
buildauthfile.goconfig.go,interface.goconfig_buildauthfile.gonpx.tmpl,uvx.tmpl,go.tmplprotocol.gobuildauthfile_test.gomock_provider.go, CLI docsThe feature requires all components to work together - splitting would result in non-functional intermediate PRs.
New CLI Commands
Supported Auth Files
Guide: Setting Up Private NPM Registry Authentication
Prerequisites
Before configuring auth files, ensure ToolHive secrets are set up:
This enables secure storage using your system keyring or encrypted file storage.
Step 1: Configure the Registry URL
Tell ToolHive to use your private registry instead of the public npmjs.org:
Step 2: Configure Authentication
NPM requires registry-scoped authentication tokens in
.npmrcformat. Set your auth token (stored securely in secrets, not config):For registries requiring basic auth or multiple registries:
Step 3: Verify Configuration
The dry-run output should show:
ENV NPM_CONFIG_REGISTRY="https://npm.corp.example.com"COPY .npmrc /root/.npmrcStep 4: Run Your Private Package
thv run 'npx://@myorg/my-private-package' --name my-serverSecurity Notes
secret:BUILD_AUTH_FILE_npmrc, not actual credentials.npmrcfile is NOT copied to the final container image--show-contentflag to display actual token valuesnpm installin the builder stage; the runtime image only contains the installed packagesCleanup
Test plan
@test/mcp-hellonpx://@test/mcp-hello🤖 Generated with Claude Code