From 922db60f6fa861a6c6f2cce3505469c35ef46479 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D7=A0=CF=85=CE=B1=CE=B7=20=D7=A0=CF=85=CE=B1=CE=B7=D1=95?= =?UTF-8?q?=CF=83=CE=B7?= Date: Thu, 17 Sep 2026 14:40:17 -0700 Subject: [PATCH 1/2] test: cover account-name guards and hash error path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI's Go job failed at 99.8% against the 99.9% gate: two recent security fixes added uncovered statements. debian_group.go gained a validateAccountName guard in GetGroup, CreateGroup, UpdateGroup, and DeleteGroup with no rejection test, and defaultHashPassword's error branch was unreachable because Generate always succeeds with a nil salt. Add an invalid-name row to each group test, mirroring the existing user-side pattern, where the gomock controller's strict expectations prove no command ran. Move the sha512crypt call behind a generateHash package variable, following the hashPassword injection convention, and add a test that injects a failing generator to cover the wrapped error. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude --- .../controller/api/node/user/export_test.go | 16 +++++++ .../controller/api/node/user/password_hash.go | 17 ++++--- .../node/user/password_hash_public_test.go | 25 +++++++++++ .../provider/node/user/debian_public_test.go | 45 +++++++++++++++++++ 4 files changed, 97 insertions(+), 6 deletions(-) diff --git a/internal/controller/api/node/user/export_test.go b/internal/controller/api/node/user/export_test.go index d29b59d7f..908be5423 100644 --- a/internal/controller/api/node/user/export_test.go +++ b/internal/controller/api/node/user/export_test.go @@ -20,6 +20,10 @@ package user +import ( + sha512crypt "github.com/GehirnInc/crypt/sha512_crypt" +) + // HashPasswordForTest exposes defaultHashPassword to external tests. func HashPasswordForTest( password string, @@ -38,3 +42,15 @@ func SetHashPasswordFn( func ResetHashPasswordFn() { hashPassword = defaultHashPassword } + +// SetGenerateHashFn overrides the underlying crypt library call for testing. +func SetGenerateHashFn( + fn func([]byte, []byte) (string, error), +) { + generateHash = fn +} + +// ResetGenerateHashFn restores the default crypt library call. +func ResetGenerateHashFn() { + generateHash = sha512crypt.New().Generate +} diff --git a/internal/controller/api/node/user/password_hash.go b/internal/controller/api/node/user/password_hash.go index da483d5eb..7e7edb07b 100644 --- a/internal/controller/api/node/user/password_hash.go +++ b/internal/controller/api/node/user/password_hash.go @@ -30,6 +30,11 @@ import ( // failure. See export_test.go for the setter/reset pair. var hashPassword = defaultHashPassword +// generateHash is the underlying crypt library call behind +// defaultHashPassword, held in a package-level variable so tests can inject +// a failure from it directly. See export_test.go for the setter/reset pair. +var generateHash = sha512crypt.New().Generate + // defaultHashPassword hashes a plaintext password into a SHA-512 crypt hash // ("$6$$"), the format /etc/shadow and "chpasswd -e" accept. The // salt is generated by the underlying library from crypto/rand. @@ -39,12 +44,12 @@ var hashPassword = defaultHashPassword func defaultHashPassword( password string, ) (string, error) { - // A nil salt tells Generate to create its own, valid by construction, so - // this error is not reachable through any password value. The check is - // kept because Generate's signature can fail, and callers that inject a - // different hasher via SetHashPasswordFn (see export_test.go) rely on - // this path being wired up. - hash, err := sha512crypt.New().Generate([]byte(password), nil) + // A nil salt tells generateHash to create its own, valid by construction, + // so this error is not reachable through any password value in + // production. The check is kept because generateHash's signature can + // fail, and SetGenerateHashFn (see export_test.go) injects a failure to + // cover it. + hash, err := generateHash([]byte(password), nil) if err != nil { return "", fmt.Errorf("hash password: %w", err) } diff --git a/internal/controller/api/node/user/password_hash_public_test.go b/internal/controller/api/node/user/password_hash_public_test.go index 16b13532b..feafef54c 100644 --- a/internal/controller/api/node/user/password_hash_public_test.go +++ b/internal/controller/api/node/user/password_hash_public_test.go @@ -21,6 +21,7 @@ package user_test import ( + "errors" "strings" "testing" @@ -33,15 +34,21 @@ type PasswordHashPublicTestSuite struct { suite.Suite } +func (s *PasswordHashPublicTestSuite) TearDownSubTest() { + apiuser.ResetGenerateHashFn() +} + func (s *PasswordHashPublicTestSuite) TestHashPassword() { tests := []struct { name string password string + setup func() validateFunc func(hash string, err error) }{ { name: "produces a sha-512 crypt hash", password: "correct horse battery staple", + setup: func() {}, validateFunc: func(hash string, err error) { s.NoError(err) s.True(strings.HasPrefix(hash, "$6$")) @@ -51,6 +58,7 @@ func (s *PasswordHashPublicTestSuite) TestHashPassword() { { name: "empty password still hashes", password: "", + setup: func() {}, validateFunc: func(hash string, err error) { s.NoError(err) s.True(strings.HasPrefix(hash, "$6$")) @@ -59,6 +67,7 @@ func (s *PasswordHashPublicTestSuite) TestHashPassword() { { name: "two calls produce different hashes", password: "same-password", + setup: func() {}, validateFunc: func(hash string, err error) { s.NoError(err) other, err := apiuser.HashPasswordForTest("same-password") @@ -66,10 +75,26 @@ func (s *PasswordHashPublicTestSuite) TestHashPassword() { s.NotEqual(hash, other, "salts must be random per call") }, }, + { + name: "when the underlying generator fails", + password: "correct horse battery staple", + setup: func() { + apiuser.SetGenerateHashFn(func(_ []byte, _ []byte) (string, error) { + return "", errors.New("salt decode failed") + }) + }, + validateFunc: func(hash string, err error) { + s.Error(err) + s.Empty(hash) + s.Contains(err.Error(), "hash password: salt decode failed") + }, + }, } for _, tc := range tests { s.Run(tc.name, func() { + tc.setup() + hash, err := apiuser.HashPasswordForTest(tc.password) tc.validateFunc(hash, err) }) diff --git a/internal/provider/node/user/debian_public_test.go b/internal/provider/node/user/debian_public_test.go index 181c19690..7e0e5de56 100644 --- a/internal/provider/node/user/debian_public_test.go +++ b/internal/provider/node/user/debian_public_test.go @@ -971,6 +971,16 @@ func (suite *DebianPublicTestSuite) TestGetGroup() { suite.Nil(result) }, }, + { + name: "when group name is invalid", + groupName: "Invalid", + groupContent: "", + validateFunc: func(result *user.Group, err error) { + suite.Error(err) + suite.Nil(result) + suite.Contains(err.Error(), "invalid group name") + }, + }, } for _, tc := range tests { @@ -1061,6 +1071,18 @@ func (suite *DebianPublicTestSuite) TestCreateGroup() { suite.Contains(err.Error(), "groupadd failed") }, }, + { + name: "when group name is invalid", + opts: user.CreateGroupOpts{ + Name: "Invalid", + }, + setup: func() {}, + validateFunc: func(result *user.GroupResult, err error) { + suite.Error(err) + suite.Nil(result) + suite.Contains(err.Error(), "invalid group name") + }, + }, } for _, tc := range tests { @@ -1116,6 +1138,19 @@ func (suite *DebianPublicTestSuite) TestUpdateGroup() { suite.Contains(err.Error(), "gpasswd failed") }, }, + { + name: "when group name is invalid", + groupName: "Invalid", + opts: user.UpdateGroupOpts{ + Members: []string{"john"}, + }, + setup: func() {}, + validateFunc: func(result *user.GroupResult, err error) { + suite.Error(err) + suite.Nil(result) + suite.Contains(err.Error(), "invalid group name") + }, + }, } for _, tc := range tests { @@ -1164,6 +1199,16 @@ func (suite *DebianPublicTestSuite) TestDeleteGroup() { suite.Contains(err.Error(), "groupdel failed") }, }, + { + name: "when group name is invalid", + groupName: "Invalid", + setup: func() {}, + validateFunc: func(result *user.GroupResult, err error) { + suite.Error(err) + suite.Nil(result) + suite.Contains(err.Error(), "invalid group name") + }, + }, } for _, tc := range tests { From 5e954ae5f899628c531297cc08368763d80a4e54 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D7=A0=CF=85=CE=B1=CE=B7=20=D7=A0=CF=85=CE=B1=CE=B7=D1=95?= =?UTF-8?q?=CF=83=CE=B7?= Date: Thu, 17 Sep 2026 15:10:03 -0700 Subject: [PATCH 2/2] test: cover account-name and password-input guards across user and group MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI's Go job failed at 99.8% against the 99.9% gate: two recent security fixes added the same guards in many places, and only the group provider and the password hash error path had rejection tests. Cover the remaining gaps: - debian_user.go: add an invalid-name row to GetUser, CreateUser, UpdateUser, and DeleteUser, mirroring the group pattern. - debian_ssh_key.go: add an invalid-username row to ListKeys, AddKey, and RemoveKey. - validate.go: expose validatePasswordInput via export_test.go and add a direct test for its name-side rejection, which CreateUser and ChangePassword can never reach since validateAccountName already rejects a colon or line break in the name before either caller gets there. - internal/controller/api/node/user handlers: add a 400 row with an invalid name in the path to every handler that calls validateName (user/group get, update, delete, password, and SSH key add/list/ delete), which also exercises internal/validation's account_name validator for the first time and closes out init()'s coverage. - PutNodeUser and PutNodeGroup additionally gained a body-level account_name check (dive over Groups/Members); add a row with an invalid entry in each to cover that branch too. Verified with `just test`: 100.0% (up from 99.8%), meets the 99.9% gate. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01FuKUsHFG1EqZXamffh9M2c --- .../api/node/user/group_delete_public_test.go | 12 +++ .../api/node/user/group_get_public_test.go | 12 +++ .../api/node/user/group_update_public_test.go | 26 ++++++ .../node/user/ssh_key_create_public_test.go | 15 ++++ .../node/user/ssh_key_delete_public_test.go | 13 +++ .../node/user/ssh_key_list_get_public_test.go | 12 +++ .../api/node/user/user_delete_public_test.go | 12 +++ .../api/node/user/user_get_public_test.go | 12 +++ .../node/user/user_password_public_test.go | 13 +++ .../api/node/user/user_update_public_test.go | 26 ++++++ .../provider/node/user/debian_public_test.go | 46 ++++++++++ .../node/user/debian_ssh_key_public_test.go | 38 ++++++++ internal/provider/node/user/export_test.go | 33 +++++++ .../node/user/validate_public_test.go | 87 +++++++++++++++++++ 14 files changed, 357 insertions(+) create mode 100644 internal/provider/node/user/export_test.go create mode 100644 internal/provider/node/user/validate_public_test.go diff --git a/internal/controller/api/node/user/group_delete_public_test.go b/internal/controller/api/node/user/group_delete_public_test.go index 90d1d7218..d99ccbf88 100644 --- a/internal/controller/api/node/user/group_delete_public_test.go +++ b/internal/controller/api/node/user/group_delete_public_test.go @@ -120,6 +120,18 @@ func (s *GroupDeletePublicTestSuite) TestDeleteNodeGroup() { s.True(ok) }, }, + { + name: "validation error invalid name", + request: gen.DeleteNodeGroupRequestObject{ + Hostname: "server1", + Name: "Invalid", + }, + setupMock: func() {}, + validateFunc: func(resp gen.DeleteNodeGroupResponseObject) { + _, ok := resp.(gen.DeleteNodeGroup400JSONResponse) + s.True(ok) + }, + }, { name: "not found", request: gen.DeleteNodeGroupRequestObject{ diff --git a/internal/controller/api/node/user/group_get_public_test.go b/internal/controller/api/node/user/group_get_public_test.go index 5e468d44b..ce5687f89 100644 --- a/internal/controller/api/node/user/group_get_public_test.go +++ b/internal/controller/api/node/user/group_get_public_test.go @@ -124,6 +124,18 @@ func (s *GroupGetPublicTestSuite) TestGetNodeGroupByName() { s.True(ok) }, }, + { + name: "validation error invalid name", + request: gen.GetNodeGroupByNameRequestObject{ + Hostname: "server1", + Name: "Invalid", + }, + setupMock: func() {}, + validateFunc: func(resp gen.GetNodeGroupByNameResponseObject) { + _, ok := resp.(gen.GetNodeGroupByName400JSONResponse) + s.True(ok) + }, + }, { name: "not found", request: gen.GetNodeGroupByNameRequestObject{ diff --git a/internal/controller/api/node/user/group_update_public_test.go b/internal/controller/api/node/user/group_update_public_test.go index f49ff8d14..2d4ec2295 100644 --- a/internal/controller/api/node/user/group_update_public_test.go +++ b/internal/controller/api/node/user/group_update_public_test.go @@ -139,6 +139,32 @@ func (s *GroupUpdatePublicTestSuite) TestPutNodeGroup() { s.True(ok) }, }, + { + name: "validation error invalid name", + request: gen.PutNodeGroupRequestObject{ + Hostname: "server1", + Name: "Invalid", + Body: &gen.GroupUpdateRequest{Members: &members}, + }, + setupMock: func() {}, + validateFunc: func(resp gen.PutNodeGroupResponseObject) { + _, ok := resp.(gen.PutNodeGroup400JSONResponse) + s.True(ok) + }, + }, + { + name: "validation error invalid member in body", + request: gen.PutNodeGroupRequestObject{ + Hostname: "server1", + Name: "devops", + Body: &gen.GroupUpdateRequest{Members: &[]string{"Invalid"}}, + }, + setupMock: func() {}, + validateFunc: func(resp gen.PutNodeGroupResponseObject) { + _, ok := resp.(gen.PutNodeGroup400JSONResponse) + s.True(ok) + }, + }, { name: "not found", request: gen.PutNodeGroupRequestObject{ diff --git a/internal/controller/api/node/user/ssh_key_create_public_test.go b/internal/controller/api/node/user/ssh_key_create_public_test.go index 5f22f6680..120ae2823 100644 --- a/internal/controller/api/node/user/ssh_key_create_public_test.go +++ b/internal/controller/api/node/user/ssh_key_create_public_test.go @@ -153,6 +153,21 @@ func (s *SSHKeyCreatePublicTestSuite) TestPostNodeUserSSHKey() { s.True(ok) }, }, + { + name: "validation error invalid name", + request: gen.PostNodeUserSSHKeyRequestObject{ + Hostname: "server1", + Name: "Invalid", + Body: &gen.SSHKeyAddRequest{ + Key: "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAITest user@host", + }, + }, + setupMock: func() {}, + validateFunc: func(resp gen.PostNodeUserSSHKeyResponseObject) { + _, ok := resp.(gen.PostNodeUserSSHKey400JSONResponse) + s.True(ok) + }, + }, { name: "when job skipped", request: gen.PostNodeUserSSHKeyRequestObject{ diff --git a/internal/controller/api/node/user/ssh_key_delete_public_test.go b/internal/controller/api/node/user/ssh_key_delete_public_test.go index 8919cd973..0cf71af70 100644 --- a/internal/controller/api/node/user/ssh_key_delete_public_test.go +++ b/internal/controller/api/node/user/ssh_key_delete_public_test.go @@ -184,6 +184,19 @@ func (s *SSHKeyDeletePublicTestSuite) TestDeleteNodeUserSSHKey() { s.Contains(*r.Error, "required") }, }, + { + name: "validation error invalid name", + request: gen.DeleteNodeUserSSHKeyRequestObject{ + Hostname: "server1", + Name: "Invalid", + Fingerprint: "SHA256:abc123", + }, + setupMock: func() {}, + validateFunc: func(resp gen.DeleteNodeUserSSHKeyResponseObject) { + _, ok := resp.(gen.DeleteNodeUserSSHKey400JSONResponse) + s.True(ok) + }, + }, { name: "broadcast target _all", request: gen.DeleteNodeUserSSHKeyRequestObject{ diff --git a/internal/controller/api/node/user/ssh_key_list_get_public_test.go b/internal/controller/api/node/user/ssh_key_list_get_public_test.go index 6a21a64e7..65d4230af 100644 --- a/internal/controller/api/node/user/ssh_key_list_get_public_test.go +++ b/internal/controller/api/node/user/ssh_key_list_get_public_test.go @@ -242,6 +242,18 @@ func (s *SSHKeyListGetPublicTestSuite) TestGetNodeUserSSHKey() { s.Contains(*r.Error, "required") }, }, + { + name: "validation error invalid name", + request: gen.GetNodeUserSSHKeyRequestObject{ + Hostname: "server1", + Name: "Invalid", + }, + setupMock: func() {}, + validateFunc: func(resp gen.GetNodeUserSSHKeyResponseObject) { + _, ok := resp.(gen.GetNodeUserSSHKey400JSONResponse) + s.True(ok) + }, + }, { name: "broadcast target _all", request: gen.GetNodeUserSSHKeyRequestObject{ diff --git a/internal/controller/api/node/user/user_delete_public_test.go b/internal/controller/api/node/user/user_delete_public_test.go index 4caeebc9a..28aa7d380 100644 --- a/internal/controller/api/node/user/user_delete_public_test.go +++ b/internal/controller/api/node/user/user_delete_public_test.go @@ -120,6 +120,18 @@ func (s *UserDeletePublicTestSuite) TestDeleteNodeUser() { s.True(ok) }, }, + { + name: "validation error invalid name", + request: gen.DeleteNodeUserRequestObject{ + Hostname: "server1", + Name: "Invalid", + }, + setupMock: func() {}, + validateFunc: func(resp gen.DeleteNodeUserResponseObject) { + _, ok := resp.(gen.DeleteNodeUser400JSONResponse) + s.True(ok) + }, + }, { name: "not found", request: gen.DeleteNodeUserRequestObject{ diff --git a/internal/controller/api/node/user/user_get_public_test.go b/internal/controller/api/node/user/user_get_public_test.go index 38cc900b5..1257927d1 100644 --- a/internal/controller/api/node/user/user_get_public_test.go +++ b/internal/controller/api/node/user/user_get_public_test.go @@ -124,6 +124,18 @@ func (s *UserGetPublicTestSuite) TestGetNodeUserByName() { s.True(ok) }, }, + { + name: "validation error invalid name", + request: gen.GetNodeUserByNameRequestObject{ + Hostname: "server1", + Name: "Invalid", + }, + setupMock: func() {}, + validateFunc: func(resp gen.GetNodeUserByNameResponseObject) { + _, ok := resp.(gen.GetNodeUserByName400JSONResponse) + s.True(ok) + }, + }, { name: "not found", request: gen.GetNodeUserByNameRequestObject{ diff --git a/internal/controller/api/node/user/user_password_public_test.go b/internal/controller/api/node/user/user_password_public_test.go index 34b7a422b..a7d3a3600 100644 --- a/internal/controller/api/node/user/user_password_public_test.go +++ b/internal/controller/api/node/user/user_password_public_test.go @@ -173,6 +173,19 @@ func (s *UserPasswordPublicTestSuite) TestPostNodeUserPassword() { s.True(ok) }, }, + { + name: "validation error invalid name", + request: gen.PostNodeUserPasswordRequestObject{ + Hostname: "server1", + Name: "Invalid", + Body: &gen.UserPasswordRequest{Password: "newpass123"}, + }, + setupMock: func() {}, + validateFunc: func(resp gen.PostNodeUserPasswordResponseObject) { + _, ok := resp.(gen.PostNodeUserPassword400JSONResponse) + s.True(ok) + }, + }, { name: "not found", request: gen.PostNodeUserPasswordRequestObject{ diff --git a/internal/controller/api/node/user/user_update_public_test.go b/internal/controller/api/node/user/user_update_public_test.go index a0767533f..2343c37d7 100644 --- a/internal/controller/api/node/user/user_update_public_test.go +++ b/internal/controller/api/node/user/user_update_public_test.go @@ -140,6 +140,32 @@ func (s *UserUpdatePublicTestSuite) TestPutNodeUser() { s.True(ok) }, }, + { + name: "validation error invalid name", + request: gen.PutNodeUserRequestObject{ + Hostname: "server1", + Name: "Invalid", + Body: &gen.UserUpdateRequest{Shell: &shell}, + }, + setupMock: func() {}, + validateFunc: func(resp gen.PutNodeUserResponseObject) { + _, ok := resp.(gen.PutNodeUser400JSONResponse) + s.True(ok) + }, + }, + { + name: "validation error invalid group in body", + request: gen.PutNodeUserRequestObject{ + Hostname: "server1", + Name: "testuser", + Body: &gen.UserUpdateRequest{Groups: &[]string{"Invalid"}}, + }, + setupMock: func() {}, + validateFunc: func(resp gen.PutNodeUserResponseObject) { + _, ok := resp.(gen.PutNodeUser400JSONResponse) + s.True(ok) + }, + }, { name: "not found", request: gen.PutNodeUserRequestObject{ diff --git a/internal/provider/node/user/debian_public_test.go b/internal/provider/node/user/debian_public_test.go index 7e0e5de56..427874c7b 100644 --- a/internal/provider/node/user/debian_public_test.go +++ b/internal/provider/node/user/debian_public_test.go @@ -343,6 +343,17 @@ func (suite *DebianPublicTestSuite) TestGetUser() { suite.Nil(result) }, }, + { + name: "when user name is invalid", + userName: "Invalid", + passwd: "", + setup: func() {}, + validateFunc: func(result *user.User, err error) { + suite.Error(err) + suite.Nil(result) + suite.Contains(err.Error(), "invalid user name") + }, + }, { name: "when groups lookup succeeds but passwd status fails", userName: "john", @@ -520,6 +531,18 @@ func (suite *DebianPublicTestSuite) TestCreateUser() { suite.Contains(err.Error(), "invalid password hash: must be a crypt hash") }, }, + { + name: "when user name is invalid", + opts: user.CreateUserOpts{ + Name: "Invalid", + }, + setup: func() {}, + validateFunc: func(result *user.Result, err error) { + suite.Error(err) + suite.Nil(result) + suite.Contains(err.Error(), "invalid user name") + }, + }, } for _, tc := range tests { @@ -658,6 +681,19 @@ func (suite *DebianPublicTestSuite) TestUpdateUser() { suite.Contains(err.Error(), "usermod failed") }, }, + { + name: "when user name is invalid", + userName: "Invalid", + opts: user.UpdateUserOpts{ + Shell: "/bin/zsh", + }, + setup: func() {}, + validateFunc: func(result *user.Result, err error) { + suite.Error(err) + suite.Nil(result) + suite.Contains(err.Error(), "invalid user name") + }, + }, } for _, tc := range tests { @@ -706,6 +742,16 @@ func (suite *DebianPublicTestSuite) TestDeleteUser() { suite.Contains(err.Error(), "userdel failed") }, }, + { + name: "when user name is invalid", + userName: "Invalid", + setup: func() {}, + validateFunc: func(result *user.Result, err error) { + suite.Error(err) + suite.Nil(result) + suite.Contains(err.Error(), "invalid user name") + }, + }, } for _, tc := range tests { diff --git a/internal/provider/node/user/debian_ssh_key_public_test.go b/internal/provider/node/user/debian_ssh_key_public_test.go index df6bc792e..4a3011214 100644 --- a/internal/provider/node/user/debian_ssh_key_public_test.go +++ b/internal/provider/node/user/debian_ssh_key_public_test.go @@ -352,6 +352,17 @@ func (suite *DebianSSHKeyPublicTestSuite) TestListKeys() { suite.Empty(keys[0].Comment) }, }, + { + name: "when username is invalid", + username: "Invalid", + skipPasswd: true, + setupFS: func() {}, + validateFunc: func(keys []user.SSHKey, err error) { + suite.Error(err) + suite.Nil(keys) + suite.Contains(err.Error(), "invalid user name") + }, + }, } for _, tc := range tests { @@ -675,6 +686,21 @@ func (suite *DebianSSHKeyPublicTestSuite) TestAddKey() { suite.True(result.Changed) }, }, + { + name: "when username is invalid", + username: "Invalid", + skipPasswd: true, + key: user.SSHKey{ + RawLine: testKey1Line, + }, + setupFS: func() {}, + setupMock: func() {}, + validateFunc: func(result *user.SSHKeyResult, err error) { + suite.Error(err) + suite.Nil(result) + suite.Contains(err.Error(), "invalid user name") + }, + }, } for _, tc := range tests { @@ -894,6 +920,18 @@ func (suite *DebianSSHKeyPublicTestSuite) TestRemoveKey() { suite.NotContains(content, testKey1Line) }, }, + { + name: "when username is invalid", + username: "Invalid", + skipPasswd: true, + fingerprint: testKey1FP, + setupFS: func() {}, + validateFunc: func(result *user.SSHKeyResult, err error) { + suite.Error(err) + suite.Nil(result) + suite.Contains(err.Error(), "invalid user name") + }, + }, } for _, tc := range tests { diff --git a/internal/provider/node/user/export_test.go b/internal/provider/node/user/export_test.go new file mode 100644 index 000000000..f753f7ff1 --- /dev/null +++ b/internal/provider/node/user/export_test.go @@ -0,0 +1,33 @@ +// Copyright (c) 2026 John Dewey + +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to +// deal in the Software without restriction, including without limitation the +// rights to use, copy, modify, merge, publish, distribute, sublicense, and/or +// sell copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: + +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. + +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING +// FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER +// DEALINGS IN THE SOFTWARE. + +package user + +// ValidatePasswordInputForTest exposes validatePasswordInput to external +// tests. Every caller validates name via validateAccountName first, which +// already rejects a colon or line break, so the name-side branch here is +// unreachable through CreateUser or ChangePassword — this lets a test reach +// it directly. +func ValidatePasswordInputForTest( + name string, + passwordHash string, +) error { + return validatePasswordInput(name, passwordHash) +} diff --git a/internal/provider/node/user/validate_public_test.go b/internal/provider/node/user/validate_public_test.go new file mode 100644 index 000000000..b4baa6ed5 --- /dev/null +++ b/internal/provider/node/user/validate_public_test.go @@ -0,0 +1,87 @@ +// Copyright (c) 2026 John Dewey + +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to +// deal in the Software without restriction, including without limitation the +// rights to use, copy, modify, merge, publish, distribute, sublicense, and/or +// sell copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: + +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. + +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING +// FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER +// DEALINGS IN THE SOFTWARE. + +package user_test + +import ( + "testing" + + "github.com/stretchr/testify/suite" + + "github.com/osapi-io/osapi/internal/provider/node/user" +) + +// ValidatePasswordInputPublicTestSuite exercises validatePasswordInput +// directly through ValidatePasswordInputForTest (see export_test.go). Its +// callers, CreateUser and ChangePassword, both validate the name via +// validateAccountName first, which already rejects a colon or line break — +// so this suite is the only way to reach the name-side branch. +type ValidatePasswordInputPublicTestSuite struct { + suite.Suite +} + +func (s *ValidatePasswordInputPublicTestSuite) TestValidatePasswordInput() { + tests := []struct { + name string + userName string + passwordHash string + validateFunc func(err error) + }{ + { + name: "when name contains a colon", + userName: "john:root", + passwordHash: "$6$abcd$deadbeef", + validateFunc: func(err error) { + s.Error(err) + s.Contains(err.Error(), "invalid user name: must not contain a colon or line break") + }, + }, + { + name: "when name contains a line break", + userName: "john\nroot:pwned", + passwordHash: "$6$abcd$deadbeef", + validateFunc: func(err error) { + s.Error(err) + s.Contains(err.Error(), "invalid user name: must not contain a colon or line break") + }, + }, + { + name: "when name and hash are valid", + userName: "john", + passwordHash: "$6$abcd$deadbeef", + validateFunc: func(err error) { + s.NoError(err) + }, + }, + } + + for _, tc := range tests { + s.Run(tc.name, func() { + err := user.ValidatePasswordInputForTest(tc.userName, tc.passwordHash) + tc.validateFunc(err) + }) + } +} + +func TestValidatePasswordInputPublicTestSuite( + t *testing.T, +) { + suite.Run(t, new(ValidatePasswordInputPublicTestSuite)) +}