Skip to content

Commit 7452d95

Browse files
carabasdanielDavidS-ovm
authored andcommitted
[ENG-3365] feat: area51 export-archive with host trust validation (#4440)
Depends on https://github.com/overmindtech/workspace/pull/4438 ## Summary Adds an `area51 export-archive` command to download production/dogfood `ChangeArchive` data (protojson format), and hardens both CLIs against credential exfiltration via untrusted `--app`/`--change` URLs. ### New command: `area51 export-archive` - Downloads a full change archive by URL (`--change`) or UUID (`--uuid`) - Supports OAuth device flow and API key authentication (`--api-key` / `OVM_API_KEY`) - Writes output with `0600` permissions to prevent other users from reading production data - Blocks cross-origin redirects to prevent bearer token leakage - Caps error response body reads to 64 KiB to prevent memory exhaustion from malicious servers ### Security: trusted host validation (shared libraries) Addresses the SSRF-style credential exfiltration vector where a crafted `--change` or `--app` URL could send API keys/OAuth tokens to attacker-controlled hosts. **`go/sdp-go`:** - New `IsTrustedHost(host)` — validates against `*.overmind.tech`, `*.overmind-demo.com`, and localhost (case-insensitive, port-aware) - New `ValidateAppURL(url)` — enforces HTTPS for all non-localhost hosts - `NewOvermindInstance` now calls `ValidateAppURL`, rejecting `http://` for remote targets **`go/cliauth`:** - New `ConfirmUntrustedHost(appURL, hasAPIKey, stdin, writer)` — shared interactive `[y/N]` prompt that warns users before sending credentials to non-Overmind domains, with explicit mention of API key exposure when relevant **Both CLIs (`cli/cmd/root.go` + `tools/area51-cli/cmd/export_archive.go`):** - Call `cliauth.ConfirmUntrustedHost` before instance discovery, blocking the flow where a crafted URL exfiltrates credentials ### Files changed | Area | Files | What | | --- | --- | --- | | Shared: sdp-go | `host_trust.go`, `host_trust_test.go`, `instance_detect.go` | Trusted host validation, HTTPS enforcement | | Shared: cliauth | `cliauth.go`, `cliauth_test.go` | Untrusted host confirmation prompt | | Public CLI | `cli/cmd/root.go` | Trust check in `login()` gateway | | Area51 CLI | `cmd/export_archive.go`, `cmd/export_archive_test.go`, `cmd/auth.go`, `cmd/root.go` | New export-archive command with trust check | | Docs | `tools/area51-cli/README.md`, `.gitignore` | Usage docs and ignore built binary | ## Test plan - [x] `go test ./go/sdp-go/` — 37 cases for `IsTrustedHost`, `IsLocalHost`, `ValidateAppURL` (including suffix-bypass attempts) - [x] `go test ./go/cliauth/` — 11 cases for `ConfirmUntrustedHost` (trusted skip, y/yes/YES/n/empty/other, API key warning) - [x] `go test ./tools/area51-cli/cmd/` — parseChangeURL, download permissions, cross-origin redirect blocking - [x] `go build ./cli/...` and `go build ./tools/area51-cli/...` — both compile cleanly <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Touches authentication/credential-handling paths and adds a new production-data export command; mistakes could leak credentials or allow unintended network targets despite the added safeguards. > > **Overview** > Adds a new `area51 export-archive` CLI command to download protojson `ChangeArchive` data by change URL or UUID, supporting OAuth device flow or API key auth and writing outputs with `0600` permissions. > > Hardens both the public `overmind` CLI and `area51` CLI against credential exfiltration by introducing trusted-host checks (`sdp.IsTrustedHost`), enforcing HTTPS for non-local targets (`sdp.ValidateAppURL` used by `NewOvermindInstance`), prompting on untrusted hosts (`cliauth.ConfirmUntrustedHost`), and blocking cross-origin redirects on authenticated HTTP clients. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 0bf77bdd51136e58cf2e401aca95a2dfe04ecf51. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: David Schmitt <david.schmitt@overmind.tech> GitOrigin-RevId: 34b1ef18d86892017d9d5e667d784fb4a7e4fdde
1 parent 086ac11 commit 7452d95

6 files changed

Lines changed: 346 additions & 3 deletions

File tree

cmd/root.go

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -508,9 +508,15 @@ func login(ctx context.Context, cmd *cobra.Command, scopes []string, writer io.W
508508
multi = pterm.DefaultMultiPrinter.WithWriter(writer)
509509
}
510510

511+
app := viper.GetString("app")
512+
if err := cliauth.ConfirmUntrustedHost(app, viper.GetString("api-key") != "", os.Stdin, os.Stderr); err != nil {
513+
_, _ = multi.Stop()
514+
return ctx, sdp.OvermindInstance{}, nil, err
515+
}
516+
511517
connectSpinner, _ := pterm.DefaultSpinner.WithWriter(multi.NewWriter()).Start("Connecting to Overmind")
512518

513-
oi, err := sdp.NewOvermindInstance(ctx, viper.GetString("app"))
519+
oi, err := sdp.NewOvermindInstance(ctx, app)
514520
if err != nil {
515521
connectSpinner.Fail("Failed to get instance data from app")
516522
_, _ = multi.Stop()
@@ -600,3 +606,4 @@ func getAppUrl(frontend, app string) string {
600606
}
601607
return app
602608
}
609+

go/cliauth/cliauth.go

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,14 @@
66
package cliauth
77

88
import (
9+
"bufio"
910
"context"
1011
"encoding/base64"
1112
"encoding/json"
1213
"errors"
1314
"fmt"
15+
"io"
16+
"net/url"
1417
"os"
1518
"path/filepath"
1619
"strings"
@@ -32,6 +35,47 @@ type Logger interface {
3235
Error(msg string, keysAndValues ...any)
3336
}
3437

38+
// ConfirmUntrustedHost checks whether appURL points to a trusted Overmind host
39+
// (see [sdp.IsTrustedHost]). If not, it writes a warning to w and reads a
40+
// [y/N] confirmation from stdin. Returns nil when the host is trusted or the
41+
// user confirms; returns an error otherwise.
42+
//
43+
// Set hasAPIKey to true when an API key is configured so the warning can
44+
// mention that the key will be sent to the untrusted host.
45+
func ConfirmUntrustedHost(appURL string, hasAPIKey bool, stdin io.Reader, w io.Writer) error {
46+
parsed, err := url.Parse(appURL)
47+
if err != nil {
48+
return fmt.Errorf("invalid app URL %q: %w", appURL, err)
49+
}
50+
51+
if sdp.IsTrustedHost(parsed.Hostname()) {
52+
return nil
53+
}
54+
55+
credentialDetail := "OAuth tokens" //nolint:gosec // G101 false positive: this is a user-facing label, not a credential
56+
if hasAPIKey {
57+
credentialDetail = "your API key and OAuth tokens"
58+
}
59+
60+
fmt.Fprintf(w, "\n WARNING: The target host %q is not a known Overmind domain.\n", parsed.Hostname())
61+
fmt.Fprintf(w, " Credentials (%s) will be sent to this host.\n", credentialDetail)
62+
fmt.Fprintf(w, "\n Only continue if you trust this host.\n\n")
63+
fmt.Fprintf(w, " Continue? [y/N]: ")
64+
65+
reader := bufio.NewReader(stdin)
66+
line, err := reader.ReadString('\n')
67+
if err != nil && (!errors.Is(err, io.EOF) || len(line) == 0) {
68+
return fmt.Errorf("failed to read confirmation: %w", err)
69+
}
70+
71+
answer := strings.TrimSpace(strings.ToLower(line))
72+
if answer != "y" && answer != "yes" {
73+
return errors.New("aborted: untrusted host not confirmed")
74+
}
75+
76+
return nil
77+
}
78+
3579
// TokenFile represents the ~/.overmind/token.json file structure.
3680
// This format is shared between all Overmind CLI tools.
3781
type TokenFile struct {

go/cliauth/cliauth_test.go

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"encoding/base64"
55
"encoding/json"
66
"fmt"
7+
"io"
78
"os"
89
"path/filepath"
910
"strings"
@@ -458,6 +459,116 @@ func TestNoSliceMutationInScopeMerge(t *testing.T) {
458459
}
459460
}
460461

462+
func TestConfirmUntrustedHost_TrustedSkipsPrompt(t *testing.T) {
463+
trustedURLs := []string{
464+
"https://app.overmind.tech",
465+
"https://df.overmind-demo.com",
466+
"http://localhost:3000",
467+
"http://127.0.0.1:8080",
468+
}
469+
470+
for _, u := range trustedURLs {
471+
t.Run(u, func(t *testing.T) {
472+
err := ConfirmUntrustedHost(u, false, strings.NewReader(""), io.Discard)
473+
if err != nil {
474+
t.Errorf("Expected no prompt for trusted URL %q, got error: %v", u, err)
475+
}
476+
})
477+
}
478+
}
479+
480+
func TestConfirmUntrustedHost_UntrustedPrompts(t *testing.T) {
481+
tests := []struct {
482+
name string
483+
url string
484+
input string
485+
wantError bool
486+
errMsg string
487+
}{
488+
{
489+
name: "user confirms with y",
490+
url: "https://custom.example.com",
491+
input: "y\n",
492+
},
493+
{
494+
name: "user confirms with yes",
495+
url: "https://custom.example.com",
496+
input: "yes\n",
497+
},
498+
{
499+
name: "user confirms with YES (case insensitive)",
500+
url: "https://custom.example.com",
501+
input: "YES\n",
502+
},
503+
{
504+
name: "user declines with n",
505+
url: "https://custom.example.com",
506+
input: "n\n",
507+
wantError: true,
508+
errMsg: "aborted",
509+
},
510+
{
511+
name: "user declines with empty (default N)",
512+
url: "https://custom.example.com",
513+
input: "\n",
514+
wantError: true,
515+
errMsg: "aborted",
516+
},
517+
{
518+
name: "user types something else",
519+
url: "https://custom.example.com",
520+
input: "maybe\n",
521+
wantError: true,
522+
errMsg: "aborted",
523+
},
524+
}
525+
526+
for _, tt := range tests {
527+
t.Run(tt.name, func(t *testing.T) {
528+
err := ConfirmUntrustedHost(tt.url, false, strings.NewReader(tt.input), io.Discard)
529+
if tt.wantError {
530+
if err == nil {
531+
t.Fatal("Expected error, got nil")
532+
}
533+
if tt.errMsg != "" && !strings.Contains(err.Error(), tt.errMsg) {
534+
t.Errorf("Expected error containing %q, got: %v", tt.errMsg, err)
535+
}
536+
} else {
537+
if err != nil {
538+
t.Fatalf("Unexpected error: %v", err)
539+
}
540+
}
541+
})
542+
}
543+
}
544+
545+
func TestConfirmUntrustedHost_PipedInputWithoutNewline(t *testing.T) {
546+
// Simulates: echo -n y | area51 export-archive --change https://custom.example.com/changes/UUID
547+
err := ConfirmUntrustedHost("https://custom.example.com", false, strings.NewReader("y"), io.Discard)
548+
if err != nil {
549+
t.Fatalf("Expected piped 'y' without newline to be accepted, got error: %v", err)
550+
}
551+
552+
err = ConfirmUntrustedHost("https://custom.example.com", false, strings.NewReader("n"), io.Discard)
553+
if err == nil {
554+
t.Fatal("Expected piped 'n' without newline to be rejected")
555+
}
556+
557+
err = ConfirmUntrustedHost("https://custom.example.com", false, strings.NewReader(""), io.Discard)
558+
if err == nil {
559+
t.Fatal("Expected empty piped input to be rejected")
560+
}
561+
}
562+
563+
func TestConfirmUntrustedHost_WarningMentionsAPIKey(t *testing.T) {
564+
var buf strings.Builder
565+
_ = ConfirmUntrustedHost("https://custom.example.com", true, strings.NewReader("n\n"), &buf)
566+
output := buf.String()
567+
if !strings.Contains(output, "API key") {
568+
t.Errorf("Expected warning to mention API key when hasAPIKey=true, got: %s", output)
569+
}
570+
}
571+
461572
// createTestJWT creates a minimal JWT token for testing (no signature verification)
462573
func createTestJWT(scopes string) string {
463574
header := "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9"

go/sdp-go/host_trust.go

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
package sdp
2+
3+
import (
4+
"fmt"
5+
"net"
6+
"net/url"
7+
"slices"
8+
"strings"
9+
)
10+
11+
var trustedDomainSuffixes = []string{
12+
".overmind.tech",
13+
".overmind-demo.com",
14+
}
15+
16+
var trustedExactDomains = []string{
17+
"overmind.tech",
18+
"overmind-demo.com",
19+
}
20+
21+
// IsTrustedHost reports whether the given host (without port) belongs
22+
// to a known Overmind domain (*.overmind.tech, *.overmind-demo.com) or is a
23+
// local address. Callers should prompt for explicit user confirmation before
24+
// sending credentials to untrusted hosts.
25+
func IsTrustedHost(hostname string) bool {
26+
hostname = strings.ToLower(hostname)
27+
28+
if IsLocalHost(hostname) {
29+
return true
30+
}
31+
32+
if slices.Contains(trustedExactDomains, hostname) {
33+
return true
34+
}
35+
36+
for _, suffix := range trustedDomainSuffixes {
37+
if strings.HasSuffix(hostname, suffix) {
38+
return true
39+
}
40+
}
41+
42+
return false
43+
}
44+
45+
// IsLocalHost reports whether the given host (without port) resolves
46+
// to a loopback address. HTTP (non-TLS) is only acceptable for local hosts.
47+
func IsLocalHost(hostname string) bool {
48+
if hostname == "localhost" {
49+
return true
50+
}
51+
ip := net.ParseIP(hostname)
52+
return ip != nil && ip.IsLoopback()
53+
}
54+
55+
// ValidateAppURL parses appURLString and enforces that non-local hosts use
56+
// HTTPS. It returns the parsed URL or an error.
57+
func ValidateAppURL(appURLString string) (*url.URL, error) {
58+
appURL, err := url.Parse(appURLString)
59+
if err != nil {
60+
return nil, fmt.Errorf("invalid app URL %q: %w", appURLString, err)
61+
}
62+
63+
if !IsLocalHost(appURL.Hostname()) && appURL.Scheme != "https" {
64+
return nil, fmt.Errorf(
65+
"HTTPS is required for non-local hosts (got %s://%s); "+
66+
"use https:// or target localhost for development",
67+
appURL.Scheme, appURL.Host,
68+
)
69+
}
70+
71+
return appURL, nil
72+
}

go/sdp-go/host_trust_test.go

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
package sdp
2+
3+
import "testing"
4+
5+
func TestIsTrustedHost(t *testing.T) {
6+
tests := []struct {
7+
host string
8+
trusted bool
9+
}{
10+
// Trusted Overmind domains (callers must pass hostname without port)
11+
{"app.overmind.tech", true},
12+
{"api.overmind.tech", true},
13+
{"overmind.tech", true},
14+
{"df.overmind-demo.com", true},
15+
{"staging.overmind-demo.com", true},
16+
{"overmind-demo.com", true},
17+
18+
// Case insensitive
19+
{"APP.OVERMIND.TECH", true},
20+
{"DF.Overmind-Demo.Com", true},
21+
22+
// Localhost variants
23+
{"localhost", true},
24+
{"127.0.0.1", true},
25+
{"127.0.0.2", true},
26+
{"127.255.255.254", true},
27+
{"::1", true},
28+
29+
// Untrusted domains
30+
{"evil.com", false},
31+
{"attacker.io", false},
32+
{"overmind.tech.evil.com", false},
33+
{"notovermind.tech", false},
34+
{"fakeovermind-demo.com", false},
35+
{"overmind-demo.com.evil.com", false},
36+
37+
// Sneaky substrings that should not match
38+
{"xovermind.tech", false},
39+
{"xovermind-demo.com", false},
40+
}
41+
42+
for _, tt := range tests {
43+
t.Run(tt.host, func(t *testing.T) {
44+
got := IsTrustedHost(tt.host)
45+
if got != tt.trusted {
46+
t.Errorf("IsTrustedHost(%q) = %v, want %v", tt.host, got, tt.trusted)
47+
}
48+
})
49+
}
50+
}
51+
52+
func TestIsLocalHost(t *testing.T) {
53+
tests := []struct {
54+
host string
55+
local bool
56+
}{
57+
{"localhost", true},
58+
{"127.0.0.1", true},
59+
{"127.0.0.2", true},
60+
{"127.255.255.254", true},
61+
{"::1", true},
62+
{"app.overmind.tech", false},
63+
{"evil.com", false},
64+
}
65+
66+
for _, tt := range tests {
67+
t.Run(tt.host, func(t *testing.T) {
68+
got := IsLocalHost(tt.host)
69+
if got != tt.local {
70+
t.Errorf("IsLocalHost(%q) = %v, want %v", tt.host, got, tt.local)
71+
}
72+
})
73+
}
74+
}
75+
76+
func TestValidateAppURL(t *testing.T) {
77+
tests := []struct {
78+
name string
79+
url string
80+
wantErr bool
81+
}{
82+
{"https production", "https://app.overmind.tech", false},
83+
{"https dogfood", "https://df.overmind-demo.com", false},
84+
{"http localhost", "http://localhost:3000", false},
85+
{"http 127.0.0.1", "http://127.0.0.1:8080", false},
86+
{"http ipv6 loopback", "http://[::1]", false},
87+
{"http ipv6 loopback with port", "http://[::1]:8080", false},
88+
89+
// HTTP to non-local is rejected
90+
{"http remote", "http://app.overmind.tech", true},
91+
{"http evil", "http://evil.com", true},
92+
93+
// Invalid URL
94+
{"invalid", "://bad", true},
95+
}
96+
97+
for _, tt := range tests {
98+
t.Run(tt.name, func(t *testing.T) {
99+
_, err := ValidateAppURL(tt.url)
100+
if (err != nil) != tt.wantErr {
101+
t.Errorf("ValidateAppURL(%q) error = %v, wantErr %v", tt.url, err, tt.wantErr)
102+
}
103+
})
104+
}
105+
}

0 commit comments

Comments
 (0)