Skip to content

feat: configure insecure DID resolution #401

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.

Sign up for GitHub

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
alanshaw merged 2 commits into from
Apr 15, 2026
Merged

feat: configure insecure DID resolution #401

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5c38a10
feat: configure insecure DID resolution
alanshaw Apr 10, 2026
9447f28
fix: define insecure DID resolution flag and bind to viper config
alanshaw Apr 15, 2026
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
7 changes: 6 additions & 1 deletion cmd/gateway/serve.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"github.com/spf13/viper"
arc "github.com/storacha/go-ds-arc"
contentcap "github.com/storacha/go-libstoracha/capabilities/space/content"
"github.com/storacha/go-libstoracha/principalresolver"
"github.com/storacha/go-ucanto/core/delegation"
"github.com/storacha/go-ucanto/did"
"github.com/storacha/go-ucanto/ucan"
Expand Down Expand Up @@ -156,7 +157,11 @@ var serveCmd = &cobra.Command{
indexer, indexerPrincipal := cmdutil.MustGetIndexClient(cfg.Network)

network := cmdutil.MustGetNetworkConfig(cfg.Network, "")
uploadServiceVerifier, err := cmdutil.ResolveDIDWebAndWrap(ctx, network.UploadID)
var resolverOpts []principalresolver.Option
if network.InsecureDIDResolution {
resolverOpts = append(resolverOpts, principalresolver.InsecureResolution())
}
uploadServiceVerifier, err := cmdutil.ResolveDIDWebAndWrap(ctx, network.UploadID, resolverOpts...)
if err != nil {
return err
}
Expand Down
7 changes: 6 additions & 1 deletion cmd/retrieve.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"github.com/mitchellh/go-wordwrap"
"github.com/spf13/cobra"
contentcap "github.com/storacha/go-libstoracha/capabilities/space/content"
"github.com/storacha/go-libstoracha/principalresolver"
"github.com/storacha/go-ucanto/core/delegation"
"github.com/storacha/go-ucanto/did"
"github.com/storacha/go-ucanto/ucan"
Expand Down Expand Up @@ -81,7 +82,11 @@ var retrieveCmd = &cobra.Command{
}()

network := cmdutil.MustGetNetworkConfig(cfg.Network, "")
uploadServiceVerifier, err := cmdutil.ResolveDIDWebAndWrap(ctx, network.UploadID)
var resolverOpts []principalresolver.Option
if network.InsecureDIDResolution {
resolverOpts = append(resolverOpts, principalresolver.InsecureResolution())
}
uploadServiceVerifier, err := cmdutil.ResolveDIDWebAndWrap(ctx, network.UploadID, resolverOpts...)
if err != nil {
return err
}
Expand Down
4 changes: 4 additions & 0 deletions cmd/root.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,10 @@ func init() {
rootCmd.PersistentFlags().String("indexer-url", "", "Indexing service URL (overrides network preset)")
cobra.CheckErr(viper.BindPFlag("network.indexer_url", rootCmd.PersistentFlags().Lookup("indexer-url")))

rootCmd.PersistentFlags().Bool("insecure-did-resolution", false, "Enable insecure DID resolution (overrides network preset)")
cobra.CheckErr(rootCmd.PersistentFlags().MarkHidden("insecure-did-resolution"))
cobra.CheckErr(viper.BindPFlag("network.insecure_did_resolution", rootCmd.PersistentFlags().Lookup("insecure-did-resolution")))

// Preparation configuration flags
rootCmd.PersistentFlags().Uint("replicas", presets.DefaultReplicas, "Number of replicas to request per shard")
cobra.CheckErr(rootCmd.PersistentFlags().MarkHidden("replicas"))
Expand Down
4 changes: 2 additions & 2 deletions internal/cmdutil/cmdutil.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,8 +242,8 @@ func ResolveSpace(c *client.Client, identifier string) (did.DID, error) {
return space.DID(), nil
}

func ResolveDIDWebAndWrap(ctx context.Context, didWeb did.DID) (principal.Verifier, error) {
resolver, err := principalresolver.NewHTTPResolver([]did.DID{didWeb})
func ResolveDIDWebAndWrap(ctx context.Context, didWeb did.DID, opts ...principalresolver.Option) (principal.Verifier, error) {
resolver, err := principalresolver.NewHTTPResolver([]did.DID{didWeb}, opts...)
if err != nil {
return nil, fmt.Errorf("creating principal resolver: %w", err)
}
Expand Down
8 changes: 8 additions & 0 deletions pkg/config/network.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ type NetworkConfig struct {
// AuthorizedRetrievals indicates whether UCAN authorized retrievals are supported.
// Use a pointer to distinguish between unset and false.
AuthorizedRetrievals *bool `mapstructure:"authorized_retrievals" toml:"authorized_retrievals"`
// InsecureDIDResolution enables HTTP (instead of HTTPS) for did:web resolution.
// NB: this should only be used for development purposes.
InsecureDIDResolution bool `mapstructure:"insecure_did_resolution" toml:"insecure_did_resolution,omitempty"`

@frrist frrist Apr 14, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the long comment here:

There is a Viper registration gap here, GUPPY_NETWORK_INSECURE_DID_RESOLUTION silently won't work, and this is a pattern I think we keep getting wrong across our projects, so I want to flag it here before it lands, not meaning to single out this PR, but it makes a good example of the problem.

Adding InsecureDIDResolution with just a mapstructure tag registers the field with our struct, but Viper has no idea the key exists until something tells it so. Viper only "knows" a key when one of these has happened:

  1. viper.SetDefault("network.insecure_did_resolution", …)
  2. viper.BindEnv("network.insecure_did_resolution")
  3. viper.BindPFlag("network.insecure_did_resolution", …)
  4. The key is physically present in a config file that Viper reads.

viper.AutomaticEnv() is misleading, it does not scan the environment for arbitrary keys. It only consults env vars for keys Viper already knows about (see spf13/viper#761, spf13/viper#584, spf13/viper#1012 for examples). And Unmarshal decodes from AllSettings(), which is built from that same known-keys registry.

What this means in practice is that commands like GUPPY_NETWORK_INSECURE_DID_RESOLUTION=true guppy retrieve are silently ignored. The key isn't registered, so Viper never looks at the env var, and Unmarshal leaves the struct field as its zero value. But, dropping insecure_did_resolution = true into ~/.config/guppy/config.toml works, because reading the file registers the key as a side effect, which then makes the env var fallback "magically" start working for subsequent runs (until the key is removed from the file again).

So the feature appears to work during manual testing with a config file, then breaks the moment someone tries to enable it via an env var in a container/CI environment which, given this is a dev/debug knob that people will reach for from a shell, is where I suspect it'll be used.

More broadly, every network.* field that has a matching BindPFlag in cmd/root.go works correctly via env vars. network.authorized_retrievals and now network.insecure_did_resolution do not, since they rely on a config file being the source of truth. I suspect we have the same latent bug in other repos where we've leaned on struct tags alone, Piri 100% has this issue in some edge cases.

I think the rule we could adopt (and document) is: every Viper-backed config field must be explicitly registered, via one of:

  1. viper.SetDefault(...) at minimum, if it's purely file/env driven, or
  2. BindPFlag against a cobra flag, preferred for anything user-facing, because it also makes the option discoverable via --help.

(I find both options to be tedious, but can't think of a better alternative, some nice-ish work arounds in the viper issues linked to above)

For this specific field I'd lean toward a hidden persistent flag (like we do for --replicas) bound with BindPFlag, so env var, flag, and config-file paths all go through the same registered key. At the very least, a SetDefault("network.insecure_did_resolution", false) in initConfig would make the env-var path work, but we'd still be missing the discoverability that a flag gives us.

@alanshaw alanshaw Apr 15, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am adding BindPFlag and MarkHidden for insecure-did-resolution. Not doing anything with authorized-retrievals since it is now legacy and can be removed - at the end of the month there is no network that setting this flag would work with.

}

// IsEmpty returns true if no network configuration fields are set.
Expand Down Expand Up @@ -140,5 +143,10 @@ func (n NetworkConfig) ToPresetConfig(baseName string) (presets.NetworkConfig, e
network.Name = "custom"
}

if n.InsecureDIDResolution {
network.InsecureDIDResolution = true
network.Name = "custom"
}

return network, nil
}
3 changes: 3 additions & 0 deletions pkg/presets/presets.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@ type NetworkConfig struct {
UploadID did.DID
UploadURL url.URL
AuthorizedRetrievals bool // Support for UCAN authorized retrievals.
// InsecureDIDResolution enables HTTP (instead of HTTPS) for did:web resolution.
// NB: this should only be used for development purposes.
InsecureDIDResolution bool
}

// Known network configurations.
Expand Down
Loading