fix(dx): repair build, update CLAUDE.md, add tests for untested paths
Some checks failed
Security Scan / security (pull_request) Failing after 9s
Test / test (pull_request) Successful in 1m44s

- Fix cmd/forge build failure: remove extra locales.FS arg from
  RegisterCommands (signature takes single CommandRegistration)
- Update CLAUDE.md error handling section to document coreerr.E()
  pattern (was outdated log.E/fmt.Errorf reference)
- Add security_test.go for agentci: SanitizePath, EscapeShellArg,
  SecureSSHCommand, MaskToken (coverage 56% → 68%)
- Add provider_handlers_test.go for pkg/api: category filter, nil
  guards, manifest/verify/sign bad requests (coverage 31% → 52%)
- Audit confirms: no fmt.Errorf or os.ReadFile/WriteFile in production
  code (only in test files)

Co-Authored-By: Virgil <virgil@lethean.io>
This commit is contained in:
Snider 2026-03-17 08:49:55 +00:00
parent 9de597a8b0
commit 10c9e23e04
4 changed files with 331 additions and 3 deletions

View file

@ -66,7 +66,7 @@ Each subsystem has different test infrastructure — see `docs/development.md` f
- **UK English**: colour, organisation, centre, licence (noun), authorise, behaviour
- **Tests**: testify assert/require, table-driven preferred, `_Good`/`_Bad`/`_Ugly` suffix naming
- **Imports**: stdlib → `forge.lthn.ai/...` → third-party, each group separated by blank line
- **Errors**: `"package.Func: context: %w"` or `log.E("package.Func", "context", err)` — no bare `fmt.Errorf`
- **Errors**: `coreerr.E("package.Func", "context", err)` via `coreerr "forge.lthn.ai/core/go-log"` — no bare `fmt.Errorf` or `errors.New`
- **Conventional commits**: `feat(forge):`, `fix(gitea):`, `test(collect):`, `docs(agentci):`, `refactor(collect):`, `chore:`
- **Co-Author trailer**: `Co-Authored-By: Virgil <virgil@lethean.io>`
- **Licence**: EUPL-1.2

122
agentci/security_test.go Normal file
View file

@ -0,0 +1,122 @@
// SPDX-Licence-Identifier: EUPL-1.2
package agentci
import (
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func TestSanitizePath_Good(t *testing.T) {
tests := []struct {
input string
want string
}{
{"simple", "simple"},
{"with-dash", "with-dash"},
{"with_underscore", "with_underscore"},
{"with.dot", "with.dot"},
{"CamelCase", "CamelCase"},
{"123", "123"},
{"path/to/file.txt", "file.txt"},
}
for _, tt := range tests {
t.Run(tt.input, func(t *testing.T) {
got, err := SanitizePath(tt.input)
require.NoError(t, err)
assert.Equal(t, tt.want, got)
})
}
}
func TestSanitizePath_Bad(t *testing.T) {
tests := []struct {
name string
input string
}{
{"spaces", "has space"},
{"special chars", "file@name"},
{"backtick", "file`name"},
{"semicolon", "file;name"},
{"pipe", "file|name"},
{"ampersand", "file&name"},
{"dollar", "file$name"},
{"parent traversal base", ".."},
{"root", "/"},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
_, err := SanitizePath(tt.input)
assert.Error(t, err)
})
}
}
func TestEscapeShellArg_Good(t *testing.T) {
tests := []struct {
input string
want string
}{
{"simple", "'simple'"},
{"with spaces", "'with spaces'"},
{"it's", "'it'\\''s'"},
{"", "''"},
}
for _, tt := range tests {
t.Run(tt.input, func(t *testing.T) {
assert.Equal(t, tt.want, EscapeShellArg(tt.input))
})
}
}
func TestSecureSSHCommand_Good(t *testing.T) {
cmd := SecureSSHCommand("host.example.com", "ls -la")
args := cmd.Args
assert.Equal(t, "ssh", args[0])
assert.Contains(t, args, "-o")
assert.Contains(t, args, "StrictHostKeyChecking=yes")
assert.Contains(t, args, "BatchMode=yes")
assert.Contains(t, args, "ConnectTimeout=10")
assert.Equal(t, "host.example.com", args[len(args)-2])
assert.Equal(t, "ls -la", args[len(args)-1])
}
func TestMaskToken_Good(t *testing.T) {
tests := []struct {
name string
input string
want string
}{
{"long token", "abcdefghijklmnop", "abcd****mnop"},
{"exactly 8", "12345678", "1234****5678"},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
assert.Equal(t, tt.want, MaskToken(tt.input))
})
}
}
func TestMaskToken_Bad(t *testing.T) {
tests := []struct {
name string
input string
}{
{"short", "abc"},
{"empty", ""},
{"seven chars", "1234567"},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
assert.Equal(t, "*****", MaskToken(tt.input))
})
}
}

View file

@ -14,11 +14,10 @@ package forge
import (
"forge.lthn.ai/core/cli/pkg/cli"
"forge.lthn.ai/core/go-scm/locales"
)
func init() {
cli.RegisterCommands(AddForgeCommands, locales.FS)
cli.RegisterCommands(AddForgeCommands)
}
// Style aliases from shared package.

View file

@ -0,0 +1,207 @@
// SPDX-Licence-Identifier: EUPL-1.2
package api_test
import (
"encoding/json"
"net/http"
"net/http/httptest"
"testing"
goapi "forge.lthn.ai/core/api"
"forge.lthn.ai/core/go-scm/marketplace"
scmapi "forge.lthn.ai/core/go-scm/pkg/api"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
// -- Marketplace: category filter ---------------------------------------------
func TestScmProvider_ListMarketplace_Category_Good(t *testing.T) {
idx := &marketplace.Index{
Version: 1,
Modules: []marketplace.Module{
{Code: "analytics", Name: "Analytics", Category: "product"},
{Code: "lint", Name: "Linter", Category: "tool"},
},
Categories: []string{"product", "tool"},
}
p := scmapi.NewProvider(idx, nil, nil, nil)
r := setupRouter(p)
w := httptest.NewRecorder()
req, _ := http.NewRequest("GET", "/api/v1/scm/marketplace?category=tool", nil)
r.ServeHTTP(w, req)
assert.Equal(t, http.StatusOK, w.Code)
var resp goapi.Response[[]marketplace.Module]
err := json.Unmarshal(w.Body.Bytes(), &resp)
require.NoError(t, err)
assert.Len(t, resp.Data, 1)
assert.Equal(t, "lint", resp.Data[0].Code)
}
// -- Marketplace: nil search results ------------------------------------------
func TestScmProvider_ListMarketplace_SearchNoResults_Good(t *testing.T) {
idx := &marketplace.Index{
Version: 1,
Modules: []marketplace.Module{
{Code: "analytics", Name: "Analytics", Category: "product"},
},
}
p := scmapi.NewProvider(idx, nil, nil, nil)
r := setupRouter(p)
w := httptest.NewRecorder()
req, _ := http.NewRequest("GET", "/api/v1/scm/marketplace?q=nonexistent", nil)
r.ServeHTTP(w, req)
assert.Equal(t, http.StatusOK, w.Code)
}
// -- GetMarketplaceItem: nil index --------------------------------------------
func TestScmProvider_GetMarketplaceItem_NilIndex_Bad(t *testing.T) {
p := scmapi.NewProvider(nil, nil, nil, nil)
r := setupRouter(p)
w := httptest.NewRecorder()
req, _ := http.NewRequest("GET", "/api/v1/scm/marketplace/anything", nil)
r.ServeHTTP(w, req)
assert.Equal(t, http.StatusNotFound, w.Code)
}
// -- Install: nil dependencies ------------------------------------------------
func TestScmProvider_InstallItem_NilDeps_Bad(t *testing.T) {
p := scmapi.NewProvider(nil, nil, nil, nil)
r := setupRouter(p)
w := httptest.NewRecorder()
req, _ := http.NewRequest("POST", "/api/v1/scm/marketplace/test/install", nil)
r.ServeHTTP(w, req)
assert.Equal(t, http.StatusServiceUnavailable, w.Code)
}
func TestScmProvider_InstallItem_NotFound_Bad(t *testing.T) {
idx := &marketplace.Index{Version: 1}
p := scmapi.NewProvider(idx, nil, nil, nil)
r := setupRouter(p)
w := httptest.NewRecorder()
req, _ := http.NewRequest("POST", "/api/v1/scm/marketplace/nonexistent/install", nil)
r.ServeHTTP(w, req)
// installer is nil, so it returns unavailable first
assert.Equal(t, http.StatusServiceUnavailable, w.Code)
}
// -- Remove: nil installer ----------------------------------------------------
func TestScmProvider_RemoveItem_NilInstaller_Bad(t *testing.T) {
p := scmapi.NewProvider(nil, nil, nil, nil)
r := setupRouter(p)
w := httptest.NewRecorder()
req, _ := http.NewRequest("DELETE", "/api/v1/scm/marketplace/test", nil)
r.ServeHTTP(w, req)
assert.Equal(t, http.StatusServiceUnavailable, w.Code)
}
// -- Update: nil installer ----------------------------------------------------
func TestScmProvider_UpdateInstalled_NilInstaller_Bad(t *testing.T) {
p := scmapi.NewProvider(nil, nil, nil, nil)
r := setupRouter(p)
w := httptest.NewRecorder()
req, _ := http.NewRequest("POST", "/api/v1/scm/installed/test/update", nil)
r.ServeHTTP(w, req)
assert.Equal(t, http.StatusServiceUnavailable, w.Code)
}
// -- Manifest endpoints: no manifest on disk ----------------------------------
func TestScmProvider_GetManifest_NoFile_Bad(t *testing.T) {
p := scmapi.NewProvider(nil, nil, nil, nil)
r := setupRouter(p)
w := httptest.NewRecorder()
req, _ := http.NewRequest("GET", "/api/v1/scm/manifest", nil)
r.ServeHTTP(w, req)
assert.Equal(t, http.StatusNotFound, w.Code)
}
func TestScmProvider_GetPermissions_NoFile_Bad(t *testing.T) {
p := scmapi.NewProvider(nil, nil, nil, nil)
r := setupRouter(p)
w := httptest.NewRecorder()
req, _ := http.NewRequest("GET", "/api/v1/scm/manifest/permissions", nil)
r.ServeHTTP(w, req)
assert.Equal(t, http.StatusNotFound, w.Code)
}
// -- Verify: bad request ------------------------------------------------------
func TestScmProvider_VerifyManifest_NoBody_Bad(t *testing.T) {
p := scmapi.NewProvider(nil, nil, nil, nil)
r := setupRouter(p)
w := httptest.NewRecorder()
req, _ := http.NewRequest("POST", "/api/v1/scm/manifest/verify", nil)
req.Header.Set("Content-Type", "application/json")
r.ServeHTTP(w, req)
assert.Equal(t, http.StatusBadRequest, w.Code)
}
// -- Sign: bad request --------------------------------------------------------
func TestScmProvider_SignManifest_NoBody_Bad(t *testing.T) {
p := scmapi.NewProvider(nil, nil, nil, nil)
r := setupRouter(p)
w := httptest.NewRecorder()
req, _ := http.NewRequest("POST", "/api/v1/scm/manifest/sign", nil)
req.Header.Set("Content-Type", "application/json")
r.ServeHTTP(w, req)
assert.Equal(t, http.StatusBadRequest, w.Code)
}
// -- Describe: count routes ---------------------------------------------------
func TestScmProvider_Describe_RouteCount_Good(t *testing.T) {
p := scmapi.NewProvider(nil, nil, nil, nil)
descs := p.Describe()
// Verify every route from RegisterRoutes is described
expectedPaths := []string{
"/marketplace",
"/marketplace/:code",
"/marketplace/:code/install",
"/manifest",
"/manifest/verify",
"/manifest/sign",
"/manifest/permissions",
"/installed",
"/installed/:code/update",
"/registry",
}
paths := make(map[string]bool)
for _, d := range descs {
paths[d.Path] = true
}
for _, ep := range expectedPaths {
assert.True(t, paths[ep], "missing description for %s", ep)
}
}