Skip to content

Commit

Permalink
Validate domain in buf registry login and logout (#3182)
Browse files Browse the repository at this point in the history
We don't do explicit validation of the domain in 'buf registry login',
which leads to some confusing error messages. Update the command to
validate the domain with netext.ValidateHostname and update
ValidateHostname to not accept URLs (it previously didn't look up the
port number so it didn't perform proper validation).
  • Loading branch information
pkwarren committed Jul 24, 2024
1 parent 8c22e90 commit 8be3d05
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import (
"github.com/bufbuild/buf/private/pkg/app/appcmd"
"github.com/bufbuild/buf/private/pkg/app/appext"
"github.com/bufbuild/buf/private/pkg/connectclient"
"github.com/bufbuild/buf/private/pkg/netext"
"github.com/bufbuild/buf/private/pkg/netrc"
"github.com/spf13/pflag"
)
Expand Down Expand Up @@ -135,6 +136,9 @@ func inner(
remote := bufconnect.DefaultRemote
if container.NumArgs() == 1 {
remote = container.Arg(0)
if _, err := netext.ValidateHostname(remote); err != nil {
return err
}
}
// Do not print unless we are prompting
if !flags.TokenStdin {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"github.com/bufbuild/buf/private/bufpkg/bufconnect"
"github.com/bufbuild/buf/private/pkg/app/appcmd"
"github.com/bufbuild/buf/private/pkg/app/appext"
"github.com/bufbuild/buf/private/pkg/netext"
"github.com/bufbuild/buf/private/pkg/netrc"
"github.com/spf13/pflag"
)
Expand Down Expand Up @@ -63,6 +64,9 @@ func run(
remote := bufconnect.DefaultRemote
if container.NumArgs() == 1 {
remote = container.Arg(0)
if _, err := netext.ValidateHostname(remote); err != nil {
return err
}
}
modified1, err := netrc.DeleteMachineForName(container, remote)
if err != nil {
Expand Down
9 changes: 7 additions & 2 deletions private/pkg/netext/netext.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"errors"
"fmt"
"net"
"strconv"
)

const (
Expand All @@ -37,8 +38,12 @@ func ValidateHostname(hostname string) (string, error) {
}

parsedHost := hostname
if host, _, err := net.SplitHostPort(hostname); err == nil {
parsedHost = host
if host, port, err := net.SplitHostPort(hostname); err == nil {
// net.SplitHostPort performs very lax validation of ports (allowing to be resolved from /etc/services).
// Only accept numeric ports here.
if portNum, err := strconv.ParseUint(port, 10, 32); err == nil && portNum <= 65535 {
parsedHost = host
}
}
if net.ParseIP(parsedHost) != nil {
// hostname is a valid IP address
Expand Down
5 changes: 5 additions & 0 deletions private/pkg/netext/netext_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,11 @@ func TestValidateHostname(t *testing.T) {
hostname: "",
isValid: false,
},
{
description: "urls are invalid",
hostname: "https://buf.build",
isValid: false,
},
}

for _, test := range tests {
Expand Down

0 comments on commit 8be3d05

Please sign in to comment.