Skip to content

Apply network permissions from registry #1041

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

Merged
merged 1 commit into from
Jul 14, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions cmd/thv/app/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import (
"github.com/stacklok/toolhive/pkg/container"
"github.com/stacklok/toolhive/pkg/container/runtime"
"github.com/stacklok/toolhive/pkg/logger"
"github.com/stacklok/toolhive/pkg/permissions"
"github.com/stacklok/toolhive/pkg/process"
"github.com/stacklok/toolhive/pkg/registry"
"github.com/stacklok/toolhive/pkg/runner"
Expand Down Expand Up @@ -114,7 +113,7 @@ func init() {
runCmd.Flags().StringVar(
&runPermissionProfile,
"permission-profile",
permissions.ProfileNetwork,
"",
"Permission profile to use (none, network, or path to JSON file)",
)
runCmd.Flags().StringArrayVarP(
Expand Down
2 changes: 1 addition & 1 deletion docs/cli/thv_run.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 0 additions & 6 deletions pkg/api/v1/workloads.go
Original file line number Diff line number Diff line change
Expand Up @@ -225,12 +225,6 @@ func (s *WorkloadRoutes) createWorkload(w http.ResponseWriter, r *http.Request)
return
}

// Mimic behavior of the CLI by defaulting to the "network" permission profile.
// TODO: Consider moving this into the run config creation logic.
if req.PermissionProfile == nil {
req.PermissionProfile = permissions.BuiltinNetworkProfile()
}

// Fetch or build the requested image
// TODO: Make verification configurable and return errors over the API.
imageURL, imageMetadata, err := retriever.GetMCPServer(
Expand Down
107 changes: 0 additions & 107 deletions pkg/runner/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
"encoding/json"
"fmt"
"io"
"strings"

"github.com/stacklok/toolhive/pkg/audit"
"github.com/stacklok/toolhive/pkg/auth"
Expand Down Expand Up @@ -256,36 +255,6 @@ func (c *RunConfig) WithPorts(port, targetPort int) (*RunConfig, error) {
return c, nil
}

// ParsePermissionProfile loads and sets the permission profile
func (c *RunConfig) ParsePermissionProfile() (*RunConfig, error) {
if c.PermissionProfile != nil {
return c, nil
}

if c.PermissionProfileNameOrPath == "" {
return c, fmt.Errorf("permission profile name or path is required")
}

var permProfile *permissions.Profile
var err error

switch c.PermissionProfileNameOrPath {
case permissions.ProfileNone, "stdio":
permProfile = permissions.BuiltinNoneProfile()
case permissions.ProfileNetwork:
permProfile = permissions.BuiltinNetworkProfile()
default:
// Try to load from file
permProfile, err = permissions.FromFile(c.PermissionProfileNameOrPath)
if err != nil {
return c, fmt.Errorf("failed to load permission profile: %v", err)
}
}

c.PermissionProfile = permProfile
return c, nil
}

// WithEnvironmentVariables parses and sets environment variables
func (c *RunConfig) WithEnvironmentVariables(envVarStrings []string) (*RunConfig, error) {
envVars, err := environment.ParseEnvironmentVariables(envVarStrings)
Expand Down Expand Up @@ -360,79 +329,3 @@ func (c *RunConfig) WithStandardLabels() *RunConfig {
labels.AddStandardLabels(c.ContainerLabels, containerName, c.BaseName, transportLabel, c.Port)
return c
}

// ProcessVolumeMounts processes volume mounts and adds them to the permission profile
func (c *RunConfig) ProcessVolumeMounts() error {
// Skip if no volumes to process
if len(c.Volumes) == 0 {
return nil
}

// Ensure permission profile is loaded
if c.PermissionProfile == nil {
if c.PermissionProfileNameOrPath == "" {
return fmt.Errorf("permission profile is required when using volume mounts")
}

// Load the permission profile from the specified name or path
_, err := c.ParsePermissionProfile()
if err != nil {
return fmt.Errorf("failed to load permission profile: %v", err)
}
}

// Create a map of existing mount targets for quick lookup
existingMounts := make(map[string]string)

// Add existing read mounts to the map
for _, m := range c.PermissionProfile.Read {
source, target, _ := m.Parse()
existingMounts[target] = source
}

// Add existing write mounts to the map
for _, m := range c.PermissionProfile.Write {
source, target, _ := m.Parse()
existingMounts[target] = source
}

// Process each volume mount
for _, volume := range c.Volumes {
// Parse read-only flag
readOnly := strings.HasSuffix(volume, ":ro")
volumeSpec := volume
if readOnly {
volumeSpec = strings.TrimSuffix(volume, ":ro")
}

// Create and parse mount declaration
mount := permissions.MountDeclaration(volumeSpec)
source, target, err := mount.Parse()
if err != nil {
return fmt.Errorf("invalid volume format: %s (%v)", volume, err)
}

// Check for duplicate mount target
if existingSource, isDuplicate := existingMounts[target]; isDuplicate {
logger.Warnf("Skipping duplicate mount target: %s (already mounted from %s)",
target, existingSource)
continue
}

// Add the mount to the appropriate permission list
if readOnly {
c.PermissionProfile.Read = append(c.PermissionProfile.Read, mount)
} else {
c.PermissionProfile.Write = append(c.PermissionProfile.Write, mount)
}

// Add to the map of existing mounts
existingMounts[target] = source

logger.Infof("Adding volume mount: %s -> %s (%s)",
source, target,
map[bool]string{true: "read-only", false: "read-write"}[readOnly])
}

return nil
}
119 changes: 105 additions & 14 deletions pkg/runner/config_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -308,24 +308,15 @@ func (b *RunConfigBuilder) validateConfig(imageMetadata *registry.ImageMetadata)
return err
}

// If we are missing the permission profile, attempt to load one from the image metadata.
if c.PermissionProfileNameOrPath == "" && c.PermissionProfile == nil && imageMetadata != nil {
permProfilePath, err := CreatePermissionProfileFile(c.Name, imageMetadata.Permissions)
if err != nil {
// Just log the error and continue with the default permission profile
logger.Warnf("Warning: failed to create permission profile file: %v", err)
} else {
// Update the permission profile path
c.PermissionProfileNameOrPath = permProfilePath
}
}
// Set permission profile (mandatory)
if _, err = c.ParsePermissionProfile(); err != nil {
// Load or default the permission profile
// NOTE: This must be done before processing volume mounts
c.PermissionProfile, err = b.loadPermissionProfile(imageMetadata)
if err != nil {
return err
}

// Process volume mounts
if err = c.ProcessVolumeMounts(); err != nil {
if err = b.processVolumeMounts(); err != nil {
return err
}

Expand Down Expand Up @@ -362,3 +353,103 @@ func (b *RunConfigBuilder) validateConfig(imageMetadata *registry.ImageMetadata)

return nil
}

func (b *RunConfigBuilder) loadPermissionProfile(imageMetadata *registry.ImageMetadata) (*permissions.Profile, error) {
// The permission profile object takes precedence over the name or path.
if b.config.PermissionProfile != nil {
return b.config.PermissionProfile, nil
}

// Try to load the permission profile by name or path.
if b.config.PermissionProfileNameOrPath != "" {
switch b.config.PermissionProfileNameOrPath {
case permissions.ProfileNone, "stdio":
return permissions.BuiltinNoneProfile(), nil
case permissions.ProfileNetwork:
return permissions.BuiltinNetworkProfile(), nil
default:
// Try to load from file
return permissions.FromFile(b.config.PermissionProfileNameOrPath)
}
}

// If a profile was not set by name or path, check the image metadata.
if imageMetadata != nil && imageMetadata.Permissions != nil {

logger.Debugf("Using registry permission profile: %v", imageMetadata.Permissions)
return imageMetadata.Permissions, nil
}

// If no metadata is available, use the network permission profile as default.
logger.Debugf("Using default permission profile: %s", permissions.ProfileNetwork)
return permissions.BuiltinNetworkProfile(), nil
}

// processVolumeMounts processes volume mounts and adds them to the permission profile
func (b *RunConfigBuilder) processVolumeMounts() error {

// Skip if no volumes to process
if len(b.config.Volumes) == 0 {
return nil
}

// Ensure permission profile is loaded
if b.config.PermissionProfile == nil {
return fmt.Errorf("permission profile is required when using volume mounts")
}

// Create a map of existing mount targets for quick lookup
existingMounts := make(map[string]string)

// Add existing read mounts to the map
for _, m := range b.config.PermissionProfile.Read {
source, target, _ := m.Parse()
existingMounts[target] = source
}

// Add existing write mounts to the map
for _, m := range b.config.PermissionProfile.Write {
source, target, _ := m.Parse()
existingMounts[target] = source
}

// Process each volume mount
for _, volume := range b.config.Volumes {
// Parse read-only flag
readOnly := strings.HasSuffix(volume, ":ro")
volumeSpec := volume
if readOnly {
volumeSpec = strings.TrimSuffix(volume, ":ro")
}

// Create and parse mount declaration
mount := permissions.MountDeclaration(volumeSpec)
source, target, err := mount.Parse()
if err != nil {
return fmt.Errorf("invalid volume format: %s (%v)", volume, err)
}

// Check for duplicate mount target
if existingSource, isDuplicate := existingMounts[target]; isDuplicate {
logger.Warnf("Skipping duplicate mount target: %s (already mounted from %s)",
target, existingSource)
continue
}

// Add the mount to the appropriate permission list
if readOnly {
b.config.PermissionProfile.Read = append(b.config.PermissionProfile.Read, mount)
} else {
b.config.PermissionProfile.Write = append(b.config.PermissionProfile.Write, mount)
}

// Add to the map of existing mounts
existingMounts[target] = source

logger.Infof("Adding volume mount: %s -> %s (%s)",
source, target,
map[bool]string{true: "read-only", false: "read-write"}[readOnly])
}

return nil
}
Loading
Loading