Skip to content

Commit 8e44686

Browse files
committed
policy: prefix all SSH validation errors with "ssh"
1 parent 33a6505 commit 8e44686

4 files changed

Lines changed: 21 additions & 20 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ HTTP API directly.
3131
### Changes
3232

3333
- Expiring or deleting a non-existent pre-auth key now returns an error instead of silently succeeding [#3324](https://github.com/juanfont/headscale/pull/3324)
34+
- SSH policy validation errors now prefix their messages with `ssh:` so failures like `ssh: users must be specified` make it clear that the problem comes from an SSH rule violation [#3343](https://github.com/juanfont/headscale/pull/3343)
3435

3536
## 0.29.1 (2026-06-18)
3637

hscontrol/policy/policy_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1317,7 +1317,7 @@ func TestSSHPolicyRules(t *testing.T) {
13171317
]
13181318
}`,
13191319
expectErr: true,
1320-
errorMessage: `"invalid" is not a valid action`,
1320+
errorMessage: `ssh: "invalid" is not a valid action`,
13211321
},
13221322
{
13231323
name: "invalid-check-period",

hscontrol/policy/v2/types.go

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -41,17 +41,17 @@ var ErrUndefinedTagReference = errors.New("references undefined tag")
4141
var (
4242
ErrSSHTagSourceToUserDest = errors.New("tags in SSH source cannot access user-owned devices")
4343
ErrSSHUserDestRequiresSameUser = errors.New("user destination requires source to contain only that same user")
44-
ErrSSHAutogroupSelfRequiresUserSource = errors.New("autogroup:self destination requires source to contain only users or groups, not tags or autogroup:tagged")
44+
ErrSSHAutogroupSelfRequiresUserSource = errors.New("ssh: autogroup:self destination requires source to contain only users or groups, not tags or autogroup:tagged")
4545
ErrSSHTagSourceToAutogroupMember = errors.New("tags in SSH source cannot access autogroup:member (user-owned devices)")
4646
ErrSSHWildcardDestination = errors.New("wildcard (*) is not supported as SSH destination")
4747
ErrSSHCheckPeriodAboveMax = errors.New("is above the max (168h)")
4848
ErrSSHCheckPeriodNegative = errors.New("must be a positive duration")
49-
ErrSSHCheckPeriodOnNonCheck = errors.New("checkPeriod is only valid with action \"check\"")
49+
ErrSSHCheckPeriodOnNonCheck = errors.New("ssh: checkPeriod is only valid with action \"check\"")
5050
ErrInvalidLocalpart = errors.New("invalid localpart format, must be localpart:*@<domain>")
51-
ErrSSHUsersMustBeSpecified = errors.New("users must be specified")
51+
ErrSSHUsersMustBeSpecified = errors.New("ssh: users must be specified")
5252
ErrSSHUserInvalid = errors.New("is not valid")
53-
ErrSSHAcceptEnvEmpty = errors.New("acceptEnv values cannot be empty")
54-
ErrSSHActionMustBeSpecified = errors.New("action must be specified")
53+
ErrSSHAcceptEnvEmpty = errors.New("ssh: acceptEnv values cannot be empty")
54+
ErrSSHActionMustBeSpecified = errors.New("ssh: action must be specified")
5555
ErrSSHActionInvalid = errors.New("is not a valid action")
5656
ErrSSHDestinationHostAlias = errors.New("invalid dst")
5757
ErrTagNameMustStartWithLetter = errors.New("tag names must start with a letter, after 'tag:'")
@@ -1639,7 +1639,7 @@ func (a *SSHAction) UnmarshalJSON(b []byte) error {
16391639
case "check":
16401640
*a = SSHActionCheck
16411641
default:
1642-
return fmt.Errorf("%q %w", str, ErrSSHActionInvalid)
1642+
return fmt.Errorf("ssh: %q %w", str, ErrSSHActionInvalid)
16431643
}
16441644

16451645
return nil
@@ -2177,7 +2177,7 @@ func validateSSHSrcDstCombination(sources SSHSrcAliases, destinations SSHDstAlia
21772177
}
21782178
// Rule: Username destination requires source to be that same single user only
21792179
if srcHasGroups || len(srcUsernames) != 1 || !srcUsernames[string(*v)] {
2180-
return fmt.Errorf("%w %q; use autogroup:self instead for same-user SSH access",
2180+
return fmt.Errorf("ssh: %w %q; use autogroup:self instead for same-user SSH access",
21812181
ErrSSHUserDestRequiresSameUser, *v)
21822182
}
21832183
case *AutoGroup:
@@ -2428,7 +2428,7 @@ func (p *Policy) validate() error {
24282428
for _, user := range ssh.Users {
24292429
switch user {
24302430
case "", "*":
2431-
errs = append(errs, fmt.Errorf("user %q %w", user, ErrSSHUserInvalid))
2431+
errs = append(errs, fmt.Errorf("ssh: user %q %w", user, ErrSSHUserInvalid))
24322432
}
24332433
}
24342434

@@ -2498,7 +2498,7 @@ func (p *Policy) validate() error {
24982498
case *Host:
24992499
// Hosts-table aliases are valid on ACL dst but
25002500
// rejected here for SSH dst.
2501-
errs = append(errs, fmt.Errorf("%w %q", ErrSSHDestinationHostAlias, string(*dst)))
2501+
errs = append(errs, fmt.Errorf("ssh: %w %q", ErrSSHDestinationHostAlias, string(*dst)))
25022502
}
25032503
}
25042504

@@ -2852,11 +2852,11 @@ func (p *SSHCheckPeriod) Validate() error {
28522852
}
28532853

28542854
if p.Duration < 0 {
2855-
return fmt.Errorf("checkPeriod %s %w", p.Duration, ErrSSHCheckPeriodNegative)
2855+
return fmt.Errorf("ssh: checkPeriod %s %w", p.Duration, ErrSSHCheckPeriodNegative)
28562856
}
28572857

28582858
if p.Duration > SSHCheckPeriodMax {
2859-
return fmt.Errorf("checkPeriod %s %w", p.Duration, ErrSSHCheckPeriodAboveMax)
2859+
return fmt.Errorf("ssh: checkPeriod %s %w", p.Duration, ErrSSHCheckPeriodAboveMax)
28602860
}
28612861

28622862
return nil

hscontrol/policy/v2/types_test.go

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -723,7 +723,7 @@ func TestUnmarshalPolicy(t *testing.T) {
723723
]
724724
}
725725
`,
726-
wantErr: `user "*" is not valid`,
726+
wantErr: `ssh: user "*" is not valid`,
727727
},
728728
{
729729
name: "ssh-with-check-period",
@@ -4763,25 +4763,25 @@ func TestSSHActionInvalidUnmarshal(t *testing.T) {
47634763
name: "uppercase rejected",
47644764
input: `"ACCEPT"`,
47654765
wantErr: ErrSSHActionInvalid,
4766-
wantMsg: `"ACCEPT" is not a valid action`,
4766+
wantMsg: `ssh: "ACCEPT" is not a valid action`,
47674767
},
47684768
{
47694769
name: "mixedcase rejected",
47704770
input: `"Accept"`,
47714771
wantErr: ErrSSHActionInvalid,
4772-
wantMsg: `"Accept" is not a valid action`,
4772+
wantMsg: `ssh: "Accept" is not a valid action`,
47734773
},
47744774
{
47754775
name: "whitespace trimmed then mixedcase rejected",
47764776
input: `" Accept"`,
47774777
wantErr: ErrSSHActionInvalid,
4778-
wantMsg: `"Accept" is not a valid action`,
4778+
wantMsg: `ssh: "Accept" is not a valid action`,
47794779
},
47804780
{
47814781
name: "unknown action rejected",
47824782
input: `"deny"`,
47834783
wantErr: ErrSSHActionInvalid,
4784-
wantMsg: `"deny" is not a valid action`,
4784+
wantMsg: `ssh: "deny" is not a valid action`,
47854785
},
47864786
}
47874787

@@ -4882,7 +4882,7 @@ func TestSSHUserTrimEndToEnd(t *testing.T) {
48824882
_, err := unmarshalPolicy([]byte(policy))
48834883
require.Error(t, err)
48844884
require.ErrorIs(t, err, ErrSSHUserInvalid)
4885-
require.Contains(t, err.Error(), `user "" is not valid`)
4885+
require.Contains(t, err.Error(), `ssh: user "" is not valid`)
48864886
})
48874887
}
48884888

@@ -5022,14 +5022,14 @@ func TestSSHCheckPeriodInvalidDuration(t *testing.T) {
50225022
}
50235023

50245024
// TestSSHCheckPeriodNegativeMessage verifies the SaaS body for the
5025-
// negative-duration case (`checkPeriod -1m0s must be a positive duration`).
5025+
// negative-duration case (`ssh: checkPeriod -1m0s must be a positive duration`).
50265026
func TestSSHCheckPeriodNegativeMessage(t *testing.T) {
50275027
p := SSHCheckPeriod{Duration: -time.Minute}
50285028

50295029
err := p.Validate()
50305030
require.Error(t, err)
50315031
require.ErrorIs(t, err, ErrSSHCheckPeriodNegative)
5032-
require.Contains(t, err.Error(), "checkPeriod -1m0s must be a positive duration")
5032+
require.Contains(t, err.Error(), "ssh: checkPeriod -1m0s must be a positive duration")
50335033
}
50345034

50355035
func TestUnmarshalGrants(t *testing.T) {

0 commit comments

Comments
 (0)