Skip to content

Conversation

@JAORMX
Copy link
Collaborator

@JAORMX JAORMX commented Dec 2, 2025

Summary

Implement secure credential handling for build environment variables during protocol builds (npx://, uvx://, go://). This allows users to reference ToolHive secrets or shell environment variables instead of storing sensitive values in the configuration file.

Closes the implementation portion of the proposal in #2859

Large PR Justification

  • Feature + Tests: ~350 lines are unit tests for the new functionality
  • Interface changes must be atomic: The Provider interface requires all implementations to be updated simultaneously to avoid build failures
  • Three-source resolution is interdependent: The --from-secret, --from-env, and literal value paths share validation, conflict detection, and build-time resolution logic that cannot be meaningfully separated

New Features

--from-secret flag

Reference a ToolHive secret by name. The secret is validated to exist at configuration time and resolved at build time.

thv secret set npm-registry "https://:token@npm.corp.example.com"
thv config set-build-env --from-secret NPM_CONFIG_REGISTRY npm-registry

--from-env flag

Read value from the shell environment at build time. Useful for CI/CD where secrets are injected via environment.

thv config set-build-env --from-env GITHUB_TOKEN
# At build time, $GITHUB_TOKEN is read from the shell

Configuration Model

Three separate sections keep sensitive values out of the config file:

build_env:
  GOPRIVATE: "github.com/myorg/*"  # Literal values only

build_env_from_secrets:
  NPM_CONFIG_REGISTRY: "npm-registry"  # Secret name, not value

build_env_from_shell:
  - GITHUB_TOKEN  # Variable name only

Security

Property Description
No cleartext secrets Config stores references, not values
Validation at config time --from-secret validates secret exists
Validation at build time Secret values checked for dangerous characters
Multi-stage isolation Credentials only in builder stage, not final image

Multi-Stage Build Isolation

All protocol build templates use multi-stage Docker builds. The BuildEnv variables are only set in the builder stage:

Template Final Stage Copies Only
npx.tmpl node_modules, package.json, package-lock.json
uvx.tmpl /opt/uv-tools
go.tmpl /app/mcp-server binary

Changes

  • pkg/config/config.go: Add BuildEnvFromSecrets and BuildEnvFromShell fields
  • pkg/config/interface.go: Add provider interface methods
  • pkg/config/buildenv.go: Add helper functions and conflict checking
  • cmd/thv/app/config_buildenv.go: Add --from-secret and --from-env flags
  • pkg/runner/protocol.go: Resolve values from all 3 sources at build time
  • pkg/config/buildenv_test.go: Add unit tests

Test plan

  • task lint passes
  • task test passes
  • Manual testing of --from-secret workflow
  • Manual testing of --from-env workflow

🤖 Generated with Claude Code

@github-actions github-actions bot added the size/XL Extra large PR: 1000+ lines changed label Dec 2, 2025
Copy link
Contributor

@github-actions github-actions bot left a 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 transformation

Alternative:

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.

@codecov
Copy link

codecov bot commented Dec 2, 2025

Codecov Report

❌ Patch coverage is 39.77273% with 106 lines in your changes missing coverage. Please review.
✅ Project coverage is 56.58%. Comparing base (a5ff43f) to head (3e5e8b7).
⚠️ Report is 5 commits behind head on main.

Files with missing lines Patch % Lines
pkg/config/interface.go 0.00% 48 Missing ⚠️
pkg/runner/protocol.go 13.04% 35 Missing and 5 partials ⚠️
pkg/config/buildenv.go 78.04% 9 Missing and 9 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2860      +/-   ##
==========================================
- Coverage   56.66%   56.58%   -0.08%     
==========================================
  Files         322      322              
  Lines       31279    31449     +170     
==========================================
+ Hits        17723    17796      +73     
- Misses      12038    12117      +79     
- Partials     1518     1536      +18     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@JAORMX JAORMX force-pushed the build-env-secrets-implementation branch from c8db694 to e44e32a Compare December 2, 2025 23:53
@github-actions github-actions bot added size/XL Extra large PR: 1000+ lines changed and removed size/XL Extra large PR: 1000+ lines changed labels Dec 2, 2025
Implement secure credential handling for build environment variables
during protocol builds (npx://, uvx://, go://). This allows users to
reference ToolHive secrets or shell environment variables instead of
storing sensitive values in the configuration file.

New features:
- `--from-secret`: Reference a ToolHive secret by name, validated at
  configuration time, resolved at build time
- `--from-env`: Read value from shell environment at build time

Configuration model:
- BuildEnvFromSecrets: maps env var names to secret names
- BuildEnvFromShell: list of env var names to read from shell

Security:
- No sensitive values stored in config file
- Secret values validated against dangerous characters
- Multi-stage builds ensure credentials don't leak to final image

Example usage:
  thv secret set npm-registry "https://:token@npm.corp.example.com"
  thv config set-build-env --from-secret NPM_CONFIG_REGISTRY npm-registry
  thv config set-build-env --from-env GITHUB_TOKEN

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@JAORMX JAORMX force-pushed the build-env-secrets-implementation branch from e44e32a to 3e5e8b7 Compare December 3, 2025 11:55
@github-actions github-actions bot added size/XL Extra large PR: 1000+ lines changed and removed size/XL Extra large PR: 1000+ lines changed labels Dec 3, 2025
@github-actions github-actions bot dismissed their stale review December 3, 2025 11:56

Large PR justification has been provided. Thank you!

@github-actions
Copy link
Contributor

github-actions bot commented Dec 3, 2025

✅ Large PR justification has been provided. The size review has been dismissed and this PR can now proceed with normal review.

@JAORMX JAORMX merged commit 0306e9c into main Dec 3, 2025
34 of 36 checks passed
@JAORMX JAORMX deleted the build-env-secrets-implementation branch December 3, 2025 16:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/XL Extra large PR: 1000+ lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants