fix: harden profile imports and release dependencies

This commit is contained in:
Andy Dixon
2026-06-15 19:39:16 +01:00
parent b2fc84846c
commit 9bbd9ceccf
20 changed files with 1493 additions and 135 deletions
+80
View File
@@ -32,6 +32,7 @@ import (
"github.com/andydixon/nexus/internal/sshconfig"
"github.com/andydixon/nexus/internal/storage"
"github.com/andydixon/nexus/internal/updater"
"github.com/andydixon/nexus/internal/validation"
"github.com/google/uuid"
)
@@ -41,6 +42,11 @@ var appVersion = "dev"
// appUpdateURL is set at build time via ldflags.
var appUpdateURL = ""
const (
maxProfileImportJSONBytes = 5 * 1024 * 1024
maxInlineSFTPUploadBytes = 25 * 1024 * 1024
)
// Service is the main API surface exposed to the frontend via Wails bindings.
type Service struct {
store *storage.Store
@@ -90,6 +96,7 @@ func (s *Service) CreateProfile(profile models.Profile) (models.Profile, error)
if profile.ID == "" {
profile.ID = uuid.New().String()
}
normaliseProfileForSave(&profile)
now := time.Now()
profile.CreatedAt = now
profile.UpdatedAt = now
@@ -98,6 +105,9 @@ func (s *Service) CreateProfile(profile models.Profile) (models.Profile, error)
profile.Tags = []string{}
}
if err := validation.ValidateProfile(&profile); err != nil {
return models.Profile{}, err
}
if err := s.store.CreateProfile(&profile); err != nil {
return models.Profile{}, fmt.Errorf("failed to create profile: %w", err)
}
@@ -116,7 +126,11 @@ func (s *Service) GetProfile(id string) (models.Profile, error) {
// UpdateProfile persists changes to an existing profile.
func (s *Service) UpdateProfile(profile models.Profile) (models.Profile, error) {
normaliseProfileForSave(&profile)
profile.UpdatedAt = time.Now()
if err := validation.ValidateProfile(&profile); err != nil {
return models.Profile{}, err
}
if err := s.store.UpdateProfile(&profile); err != nil {
return models.Profile{}, fmt.Errorf("failed to update profile: %w", err)
}
@@ -699,11 +713,21 @@ func (s *Service) ExportProfiles(includeSecrets bool) (string, error) {
// - "overwrite" replaces existing profiles
// - "duplicate" always creates a new profile with a fresh ID
func (s *Service) ImportProfiles(jsonData string, strategy string) (int, error) {
if len(jsonData) > maxProfileImportJSONBytes {
return 0, fmt.Errorf("import data is too large (max %d bytes)", maxProfileImportJSONBytes)
}
var profiles []*models.Profile
if err := json.Unmarshal([]byte(jsonData), &profiles); err != nil {
return 0, fmt.Errorf("invalid JSON: %w", err)
}
if err := validateImportStrategy(strategy); err != nil {
return 0, err
}
if err := validateImportProfiles(profiles); err != nil {
return 0, err
}
imported := 0
for _, p := range profiles {
switch strategy {
@@ -1154,6 +1178,17 @@ func (s *Service) ImportProfilesEncrypted(password string, strategy string) (int
return 0, err
}
if err := validateImportStrategy(strategy); err != nil {
return 0, err
}
profilePtrs := make([]*models.Profile, len(profiles))
for i := range profiles {
profilePtrs[i] = &profiles[i]
}
if err := validateImportProfiles(profilePtrs); err != nil {
return 0, err
}
// Import using the existing strategy logic.
imported := 0
for _, p := range profiles {
@@ -1302,11 +1337,17 @@ func (s *Service) SFTPUploadData(sessionID string, remoteDir string, fileName st
if base64Data == "" {
return fmt.Errorf("empty upload payload")
}
if base64.StdEncoding.DecodedLen(len(base64Data)) > maxInlineSFTPUploadBytes {
return fmt.Errorf("upload payload is too large (max %d bytes)", maxInlineSFTPUploadBytes)
}
data, err := base64.StdEncoding.DecodeString(base64Data)
if err != nil {
return fmt.Errorf("failed to decode upload data: %w", err)
}
if len(data) > maxInlineSFTPUploadBytes {
return fmt.Errorf("upload payload is too large (max %d bytes)", maxInlineSFTPUploadBytes)
}
safeName := filepath.Base(fileName)
if safeName == "" || safeName == "." || safeName == "/" {
@@ -1710,6 +1751,45 @@ func validateSSHKeyName(name string) (string, error) {
return name, nil
}
func validateImportStrategy(strategy string) error {
switch strategy {
case "skip", "overwrite", "duplicate":
return nil
default:
return fmt.Errorf("unknown import strategy %q (expected skip, overwrite, or duplicate)", strategy)
}
}
func validateImportProfiles(profiles []*models.Profile) error {
for i, p := range profiles {
if p == nil {
return fmt.Errorf("profile import entry %d is null", i)
}
normaliseProfileForSave(p)
if err := validation.ValidateProfile(p); err != nil {
return fmt.Errorf("profile import entry %d is invalid: %w", i, err)
}
}
return nil
}
func normaliseProfileForSave(profile *models.Profile) {
if profile == nil {
return
}
if profile.Tags == nil {
profile.Tags = []string{}
}
if profile.VNC != nil {
if profile.VNC.SecretMode == "prompt" {
profile.VNC.SecretMode = models.SecretModeAskEveryTime
}
if profile.VNC.SSHSecretMode == "prompt" {
profile.VNC.SSHSecretMode = models.SecretModeAskEveryTime
}
}
}
func redactProfileSecrets(profiles []*models.Profile) []*models.Profile {
redacted := make([]*models.Profile, len(profiles))
for i, profile := range profiles {
+57
View File
@@ -1,11 +1,14 @@
package api
import (
"encoding/base64"
"path/filepath"
"strings"
"testing"
"github.com/andydixon/nexus/internal/audit"
"github.com/andydixon/nexus/internal/models"
"github.com/andydixon/nexus/internal/storage"
)
func TestSSHKeyPairPathsAcceptsSafeName(t *testing.T) {
@@ -65,3 +68,57 @@ func TestRedactProfileSecretsClearsInlineSSHSecrets(t *testing.T) {
t.Fatal("redaction mutated the original profile")
}
}
func TestCreateProfileRejectsUnsafeGitHubBaseURL(t *testing.T) {
svc := newTestService(t)
profile := models.NewProfile(models.ProtocolGitHub)
profile.DisplayName = "Unsafe GitHub"
profile.GitHub.BaseURL = "http://evil.example/api/v3"
if _, err := svc.CreateProfile(profile); err == nil {
t.Fatal("expected unsafe GitHub base URL to be rejected")
}
}
func TestImportProfilesRejectsNullEntry(t *testing.T) {
svc := newTestService(t)
_, err := svc.ImportProfiles(`[null]`, "skip")
if err == nil {
t.Fatal("expected null import entry to be rejected")
}
if !strings.Contains(err.Error(), "entry 0 is null") {
t.Fatalf("error = %q, want null entry detail", err.Error())
}
}
func TestImportProfilesRejectsOversizedJSON(t *testing.T) {
svc := newTestService(t)
oversized := strings.Repeat(" ", maxProfileImportJSONBytes+1)
_, err := svc.ImportProfiles(oversized, "skip")
if err == nil {
t.Fatal("expected oversized import JSON to be rejected")
}
}
func TestSFTPUploadDataRejectsOversizedPayloadBeforeSessionLookup(t *testing.T) {
svc := newTestService(t)
payload := base64.StdEncoding.EncodeToString(make([]byte, maxInlineSFTPUploadBytes+1))
err := svc.SFTPUploadData("missing-session", "/", "large.bin", payload)
if err == nil {
t.Fatal("expected oversized upload payload to be rejected")
}
if !strings.Contains(err.Error(), "too large") {
t.Fatalf("error = %q, want too large", err.Error())
}
}
func newTestService(t *testing.T) *Service {
t.Helper()
store, err := storage.NewStore(filepath.Join(t.TempDir(), "test.db"))
if err != nil {
t.Fatalf("NewStore() error = %v", err)
}
t.Cleanup(func() { store.Close() })
return NewService(store, nil, nil, nil, audit.NewLogger(store), nil)
}
+10
View File
@@ -33,6 +33,8 @@ const (
saltLength = 32
nonceLength = 12 // AES-GCM standard nonce size
maxEncryptedImportBytes = 10 * 1024 * 1024
)
// ExportBundle is the cleartext payload that gets encrypted.
@@ -131,6 +133,14 @@ func DecryptAndImport(filePath, password string) ([]models.Profile, error) {
return nil, errors.New("exportimport: password cannot be empty")
}
info, err := os.Stat(filePath)
if err != nil {
return nil, fmt.Errorf("exportimport: failed to stat file: %w", err)
}
if info.Size() > maxEncryptedImportBytes {
return nil, fmt.Errorf("exportimport: file is too large (max %d bytes)", maxEncryptedImportBytes)
}
data, err := os.ReadFile(filePath)
if err != nil {
return nil, fmt.Errorf("exportimport: failed to read file: %w", err)
@@ -129,3 +129,24 @@ func TestDecryptCorruptFile(t *testing.T) {
t.Fatal("expected error for corrupt file, got nil")
}
}
func TestDecryptRejectsOversizedFile(t *testing.T) {
dir := t.TempDir()
path := filepath.Join(dir, "huge.nexus")
f, err := os.Create(path)
if err != nil {
t.Fatal(err)
}
if err := f.Truncate(maxEncryptedImportBytes + 1); err != nil {
f.Close()
t.Fatal(err)
}
if err := f.Close(); err != nil {
t.Fatal(err)
}
_, err = DecryptAndImport(path, "password")
if err == nil {
t.Fatal("expected error for oversized file, got nil")
}
}
+3 -3
View File
@@ -69,10 +69,10 @@ func (c *Client) sshConf() models.SSHConfig {
return models.SSHConfig{}
}
// Connect establishes the SSH connection using the default (insecure)
// host-key callback. Prefer ConnectWithHostKeyCallback for production use.
// Connect refuses to connect without an explicit host-key callback. Use
// ConnectWithHostKeyCallback so all callers make host-key policy visible.
func (c *Client) Connect(getPassword func() (string, error), getPassphrase func() (string, error)) error {
return c.connectInternal(ssh.InsecureIgnoreHostKey(), getPassword, getPassphrase)
return fmt.Errorf("sshclient: host key callback is required; use ConnectWithHostKeyCallback")
}
// ConnectWithHostKeyCallback applies a custom HostKeyCallback (e.g. from
+10
View File
@@ -9,6 +9,16 @@ import (
"golang.org/x/crypto/ssh"
)
func TestConnectRequiresHostKeyCallback(t *testing.T) {
profile := models.NewProfile(models.ProtocolSSH)
client := NewClient(&profile)
err := client.Connect(nil, nil)
if err == nil {
t.Fatal("expected Connect to reject missing host key callback")
}
}
func TestJumpHostClientConfigUsesFinalHostKeyCallback(t *testing.T) {
called := false
hostKeyCallback := func(hostname string, _ net.Addr, _ ssh.PublicKey) error {
+12 -3
View File
@@ -561,9 +561,7 @@ func TestImportDuplicateHandling(t *testing.T) {
func TestImportInvalidStrategy(t *testing.T) {
store := setupTestStore(t)
// The strategy check is inside the per-profile loop, so we need at least
// one profile in the import data for it to trigger.
data := []byte(`[{"id":"x","displayName":"X","protocol":"ssh","host":"h","port":22,"tags":[],"createdAt":"2024-01-01T00:00:00Z","updatedAt":"2024-01-01T00:00:00Z"}]`)
data := []byte(`[]`)
_, err := store.ImportProfiles(data, "badstrategy")
if err == nil {
t.Error("expected error for invalid import strategy")
@@ -578,6 +576,17 @@ func TestImportInvalidJSON(t *testing.T) {
}
}
func TestImportRejectsNullProfileEntry(t *testing.T) {
store := setupTestStore(t)
_, err := store.ImportProfiles([]byte(`[null]`), "skip")
if err == nil {
t.Fatal("expected error for null profile entry")
}
if !strings.Contains(err.Error(), "entry 0 is null") {
t.Fatalf("error = %q, want null entry detail", err.Error())
}
}
// ---------------------------------------------------------------------------
// Migration idempotency
// ---------------------------------------------------------------------------
+11 -3
View File
@@ -363,13 +363,23 @@ func (s *Store) ExportProfiles(includeSecrets bool) ([]byte, error) {
// and "duplicate" creates new entries with fresh IDs. Returns the number of
// profiles imported.
func (s *Store) ImportProfiles(data []byte, strategy string) (int, error) {
switch strategy {
case "skip", "overwrite", "duplicate":
default:
return 0, fmt.Errorf("storage: unknown import strategy %q", strategy)
}
var profiles []*models.Profile
if err := json.Unmarshal(data, &profiles); err != nil {
return 0, fmt.Errorf("storage: invalid import data: %w", err)
}
imported := 0
for _, p := range profiles {
for i, p := range profiles {
if p == nil {
return imported, fmt.Errorf("storage: import profile entry %d is null", i)
}
existing, _ := s.GetProfile(p.ID)
switch strategy {
@@ -391,8 +401,6 @@ func (s *Store) ImportProfiles(data []byte, strategy string) (int, error) {
p.CreatedAt = time.Now()
p.UpdatedAt = time.Now()
}
default:
return 0, fmt.Errorf("storage: unknown import strategy %q", strategy)
}
if err := s.CreateProfile(p); err != nil {
+121 -8
View File
@@ -5,6 +5,7 @@ package validation
import (
"fmt"
"net"
"net/url"
"strings"
"github.com/andydixon/nexus/internal/models"
@@ -50,21 +51,20 @@ func (e *ValidationErrors) Add(field, message string) {
func ValidateProfile(p *models.Profile) error {
errs := &ValidationErrors{}
if p == nil {
return &ValidationError{Field: "profile", Message: "profile must not be nil"}
}
if strings.TrimSpace(p.ID) == "" {
errs.Add("id", "profile ID must not be empty")
}
if strings.TrimSpace(p.DisplayName) == "" {
errs.Add("displayName", "display name must not be empty")
}
if err := ValidateHost(p.Host); err != nil {
errs.Add("host", err.(*ValidationError).Message)
}
if err := ValidatePort(p.Port); err != nil {
errs.Add("port", err.(*ValidationError).Message)
}
switch p.Protocol {
case models.ProtocolSSH:
validateProfileHostPort(errs, p.Host, p.Port)
if strings.TrimSpace(p.Username) == "" {
errs.Add("username", "username is required for SSH connections")
} else if err := ValidateUsername(p.Username); err != nil {
@@ -78,6 +78,7 @@ func ValidateProfile(p *models.Profile) error {
}
}
case models.ProtocolTelnet:
validateProfileHostPort(errs, p.Host, p.Port)
if p.Telnet == nil {
errs.Add("telnet", "Telnet configuration is required for Telnet protocol")
} else if err := ValidateTelnetConfig(p.Telnet); err != nil {
@@ -86,6 +87,7 @@ func ValidateProfile(p *models.Profile) error {
}
}
case models.ProtocolRAW:
validateProfileHostPort(errs, p.Host, p.Port)
if p.RAW == nil {
errs.Add("raw", "RAW configuration is required for RAW protocol")
} else if err := ValidateRAWConfig(p.RAW); err != nil {
@@ -94,9 +96,21 @@ func ValidateProfile(p *models.Profile) error {
}
}
case models.ProtocolGitHub:
// GitHub profiles require no host-level validation beyond the common checks.
if p.GitHub == nil {
errs.Add("github", "GitHub configuration is required for GitHub protocol")
} else if err := ValidateGitHubConfig(p.GitHub); err != nil {
if ve, ok := err.(*ValidationErrors); ok {
errs.Errors = append(errs.Errors, ve.Errors...)
}
}
case models.ProtocolVNC:
// VNC profiles use vnc.vncHost or the profile-level host; validated at connect time.
if p.VNC == nil {
errs.Add("vnc", "VNC configuration is required for VNC protocol")
} else if err := ValidateVNCConfig(p.Host, p.VNC); err != nil {
if ve, ok := err.(*ValidationErrors); ok {
errs.Errors = append(errs.Errors, ve.Errors...)
}
}
default:
errs.Add("protocol", fmt.Sprintf("unsupported protocol %q", p.Protocol))
}
@@ -120,6 +134,15 @@ func ValidateProfile(p *models.Profile) error {
return nil
}
func validateProfileHostPort(errs *ValidationErrors, host string, port int) {
if err := ValidateHost(host); err != nil {
errs.Add("host", err.(*ValidationError).Message)
}
if err := ValidatePort(port); err != nil {
errs.Add("port", err.(*ValidationError).Message)
}
}
// ValidateHost ensures the host string is non-empty and syntactically valid.
func ValidateHost(host string) error {
h := strings.TrimSpace(host)
@@ -298,3 +321,93 @@ func ValidateRAWConfig(cfg *models.RAWConfig) error {
}
return nil
}
// ValidateGitHubConfig validates GitHub API settings before tokens are sent.
func ValidateGitHubConfig(cfg *models.GitHubConfig) error {
errs := &ValidationErrors{}
if cfg == nil {
return &ValidationError{Field: "github", Message: "GitHub configuration must not be nil"}
}
u, err := url.Parse(strings.TrimSpace(cfg.BaseURL))
if err != nil || u.Scheme == "" || u.Host == "" {
errs.Add("github.baseURL", "API base URL must be an absolute HTTPS URL")
} else {
if u.User != nil {
errs.Add("github.baseURL", "API base URL must not include credentials")
}
if u.Scheme != "https" && !(u.Scheme == "http" && isLoopbackHost(u.Hostname())) {
errs.Add("github.baseURL", "API base URL must use HTTPS unless it points to loopback")
}
}
validSecret := map[models.SecretMode]bool{
models.SecretModeSaved: true, models.SecretModeSessionOnly: true, models.SecretModeAskEveryTime: true,
}
if !validSecret[cfg.SecretMode] {
errs.Add("github.secretMode", fmt.Sprintf("unsupported secret mode %q", cfg.SecretMode))
}
if cfg.RefreshIntervalSecs != 0 && cfg.RefreshIntervalSecs < 60 {
errs.Add("github.refreshIntervalSecs", "refresh interval must be at least 60 seconds")
}
if errs.HasErrors() {
return errs
}
return nil
}
// ValidateVNCConfig validates VNC connection settings.
func ValidateVNCConfig(profileHost string, cfg *models.VNCConfig) error {
errs := &ValidationErrors{}
if cfg == nil {
return &ValidationError{Field: "vnc", Message: "VNC configuration must not be nil"}
}
vncHost := strings.TrimSpace(cfg.VNCHost)
if vncHost == "" {
vncHost = strings.TrimSpace(profileHost)
}
if err := ValidateHost(vncHost); err != nil {
errs.Add("vnc.vncHost", err.(*ValidationError).Message)
}
if err := ValidatePort(cfg.VNCPort); err != nil {
errs.Add("vnc.vncPort", err.(*ValidationError).Message)
}
if cfg.Quality < 0 || cfg.Quality > 9 {
errs.Add("vnc.quality", "quality must be between 0 and 9")
}
if cfg.SSHTunnel {
if err := ValidateHost(cfg.SSHHost); err != nil {
errs.Add("vnc.sshHost", err.(*ValidationError).Message)
}
if err := ValidatePort(cfg.SSHPort); err != nil {
errs.Add("vnc.sshPort", err.(*ValidationError).Message)
}
if strings.TrimSpace(cfg.SSHUsername) == "" {
errs.Add("vnc.sshUsername", "SSH username is required when tunnelling")
} else if err := ValidateUsername(cfg.SSHUsername); err != nil {
errs.Add("vnc.sshUsername", err.(*ValidationError).Message)
}
if cfg.SSHAuthMethod != "password" && cfg.SSHAuthMethod != "publickey" {
errs.Add("vnc.sshAuthMethod", fmt.Sprintf("unsupported SSH auth method %q", cfg.SSHAuthMethod))
}
}
if errs.HasErrors() {
return errs
}
return nil
}
func isLoopbackHost(host string) bool {
h := strings.ToLower(strings.TrimSpace(host))
if h == "localhost" {
return true
}
ip := net.ParseIP(h)
return ip != nil && ip.IsLoopback()
}
+142
View File
@@ -127,6 +127,14 @@ func TestValidateProfile(t *testing.T) {
return &p
}
validVNC := func() *models.Profile {
p := models.NewProfile(models.ProtocolVNC)
p.DisplayName = "Test VNC"
p.Host = ""
p.VNC.VNCHost = "desktop.local"
return &p
}
tests := []struct {
name string
profile *models.Profile
@@ -135,6 +143,8 @@ func TestValidateProfile(t *testing.T) {
{name: "full valid SSH profile", profile: validSSH(), wantErr: false},
{name: "full valid Telnet profile", profile: validTelnet(), wantErr: false},
{name: "full valid RAW profile", profile: validRAW(), wantErr: false},
{name: "valid VNC profile with VNC-specific host", profile: validVNC(), wantErr: false},
{name: "nil profile", profile: nil, wantErr: true},
{
name: "missing host",
profile: func() *models.Profile {
@@ -210,6 +220,138 @@ func TestValidateProfile(t *testing.T) {
}
}
// ---------------------------------------------------------------------------
// ValidateGitHubConfig
// ---------------------------------------------------------------------------
func TestValidateGitHubConfig(t *testing.T) {
tests := []struct {
name string
cfg *models.GitHubConfig
wantErr bool
}{
{
name: "valid github.com API URL",
cfg: &models.GitHubConfig{
BaseURL: "https://api.github.com",
SecretMode: models.SecretModeSaved,
RefreshIntervalSecs: 300,
},
wantErr: false,
},
{
name: "valid loopback HTTP URL",
cfg: &models.GitHubConfig{
BaseURL: "http://localhost:3000/api/v3",
SecretMode: models.SecretModeAskEveryTime,
RefreshIntervalSecs: 60,
},
wantErr: false,
},
{
name: "rejects non-HTTPS remote URL",
cfg: &models.GitHubConfig{
BaseURL: "http://evil.example/api/v3",
SecretMode: models.SecretModeSaved,
RefreshIntervalSecs: 300,
},
wantErr: true,
},
{
name: "rejects URL credentials",
cfg: &models.GitHubConfig{
BaseURL: "https://token@example.com/api/v3",
SecretMode: models.SecretModeSaved,
RefreshIntervalSecs: 300,
},
wantErr: true,
},
{
name: "rejects aggressive refresh interval",
cfg: &models.GitHubConfig{
BaseURL: "https://api.github.com",
SecretMode: models.SecretModeSaved,
RefreshIntervalSecs: 1,
},
wantErr: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
err := ValidateGitHubConfig(tt.cfg)
if (err != nil) != tt.wantErr {
t.Errorf("ValidateGitHubConfig() error = %v, wantErr %v", err, tt.wantErr)
}
})
}
}
// ---------------------------------------------------------------------------
// ValidateVNCConfig
// ---------------------------------------------------------------------------
func TestValidateVNCConfig(t *testing.T) {
tests := []struct {
name string
profileHost string
cfg *models.VNCConfig
wantErr bool
}{
{
name: "uses VNC-specific host",
profileHost: "",
cfg: &models.VNCConfig{
VNCHost: "desktop.local",
VNCPort: 5900,
Quality: 6,
},
wantErr: false,
},
{
name: "uses profile host fallback",
profileHost: "desktop.local",
cfg: &models.VNCConfig{
VNCPort: 5900,
Quality: 6,
},
wantErr: false,
},
{
name: "rejects missing host",
profileHost: "",
cfg: &models.VNCConfig{
VNCPort: 5900,
Quality: 6,
},
wantErr: true,
},
{
name: "rejects invalid SSH tunnel auth method",
profileHost: "desktop.local",
cfg: &models.VNCConfig{
VNCPort: 5900,
Quality: 6,
SSHTunnel: true,
SSHHost: "jump.local",
SSHPort: 22,
SSHUsername: "admin",
SSHAuthMethod: "token",
},
wantErr: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
err := ValidateVNCConfig(tt.profileHost, tt.cfg)
if (err != nil) != tt.wantErr {
t.Errorf("ValidateVNCConfig() error = %v, wantErr %v", err, tt.wantErr)
}
})
}
}
// ---------------------------------------------------------------------------
// ValidateSSHConfig
// ---------------------------------------------------------------------------