Skip to content

Commit e83b453

Browse files
author
Mateusz
committed
fix: address PR #6 review (auth policy, gitleaks, events, SDK redaction, tests)
- PolicyAuthenticator: api_key_sso with nil Remote returns api_key_sso_misconfigured - Widen gitleaks testdata allowlist; gitleaks git pre-commit; Windows hooks-install.ps1 - Reject BuildOptions.AuthEventSink when event_delivery is not custom - Move auth leak fixtures to testkit; opaque subtest names; InboundCallMeta String/GoString - archtest: lipsdk/auth closure uses go list -deps; listener ErrMalformedListenAddress - auth_compose: lowercase renderer keys; local_noop OnOSIdentityFallback + bundle slog - Config sentinels, accessmode kinds, challenge cap constant, osidentity/USER/USERNAME, execview - pluginreg: authErrorRenderers init, RegisterAuthErrorRenderer id order; bootlog error wrap - Minor: default listen constant, doc quotes, conformance lipAuthProvider rename Made-with: Cursor
1 parent 54071c5 commit e83b453

47 files changed

Lines changed: 389 additions & 125 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.gitleaks.toml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ title = "go-llm-interactive-proxy gitleaks config"
77
useDefault = true
88

99
[[allowlists]]
10-
description = "Test fixtures and golden files"
10+
description = "Test fixtures and golden files (any package testdata/ segment)"
1111
paths = [
12-
'''^testdata/''',
12+
'''(^|/)testdata/''',
1313
]

Makefile

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,4 +132,8 @@ run:
132132
$(GO) run ./cmd/lipstd --config ./config/config.yaml
133133

134134
hooks-install:
135+
ifeq ($(OS),Windows_NT)
136+
@powershell -NoProfile -ExecutionPolicy Bypass -File scripts/install-hooks.ps1
137+
else
135138
@bash scripts/install-hooks.sh
139+
endif

cmd/lipstd/bootlog.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package main
22

33
import (
44
"context"
5+
"fmt"
56
"log/slog"
67

78
"github.com/matdev83/go-llm-interactive-proxy/internal/core/config"
@@ -14,7 +15,7 @@ func logBootstrapAccessAuth(ctx context.Context, log *slog.Logger, cfg *config.C
1415
accessMode, err := cfg.EffectiveAccessMode()
1516
if err != nil {
1617
log.ErrorContext(ctx, "lipstd: resolve effective access mode", "error", err)
17-
return err
18+
return fmt.Errorf("lipstd: bootstrap access/auth: %w", err)
1819
}
1920
effHandler, effLevel := cfg.EffectiveAuthForAudit()
2021
log.InfoContext(ctx, "lipstd: effective access and authentication",

internal/archtest/contract_lipapi_auth_imports_test.go

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -21,20 +21,21 @@ func TestPkgLipapiDoesNotImportAuthSDKPackages(t *testing.T) {
2121
assertGoListImportsExclude(t, "./pkg/lipapi", forbidden)
2222
}
2323

24+
// lipsdkAuthImportClosureForbidden is matched against every ImportPath in `go list -deps` for
25+
// ./pkg/lipsdk/auth (transitive closure, same policy as the former direct-imports-only check).
26+
var lipsdkAuthImportClosureForbidden = []forbiddenDep{
27+
{Substr: "/internal/core/", ErrMsg: "pkg/lipsdk/auth must not depend on internal/core (transitively)"},
28+
{Substr: "/internal/plugins/", ErrMsg: "pkg/lipsdk/auth must not depend on internal/plugins (transitively)"},
29+
{Substr: "github.com/openai/openai-go", ErrMsg: "pkg/lipsdk/auth must not depend on OpenAI Go SDK (transitively)"},
30+
{Substr: "github.com/anthropics/anthropic-sdk-go", ErrMsg: "pkg/lipsdk/auth must not depend on Anthropic SDK (transitively)"},
31+
{Substr: "github.com/aws/aws-sdk-go-v2", ErrMsg: "pkg/lipsdk/auth must not depend on AWS SDKs (transitively)"},
32+
}
33+
2434
// TestPkgLipsdkAuthImportClosure_staysSDKLocal ensures public auth DTOs do not pull core, plugins,
2535
// or provider SDKs into the stable auth package (task 10.4, complements task 1.4).
2636
func TestPkgLipsdkAuthImportClosure_staysSDKLocal(t *testing.T) {
2737
t.Parallel()
28-
forbidden := []struct {
29-
sub, msg string
30-
}{
31-
{"/internal/core/", "pkg/lipsdk/auth must not import internal/core"},
32-
{"/internal/plugins/", "pkg/lipsdk/auth must not import concrete protocol plugins"},
33-
{"github.com/openai/openai-go", "pkg/lipsdk/auth must not import OpenAI provider SDK"},
34-
{"github.com/anthropics/anthropic-sdk-go", "pkg/lipsdk/auth must not import Anthropic SDK"},
35-
{"github.com/aws/aws-sdk-go-v2", "pkg/lipsdk/auth must not import AWS SDKs"},
36-
}
37-
assertGoListImportsExclude(t, "./pkg/lipsdk/auth", forbidden)
38+
assertDepsExcludeForbidden(t, []string{"./pkg/lipsdk/auth"}, lipsdkAuthImportClosureForbidden)
3839
}
3940

4041
func assertGoListImportsExclude(t *testing.T, goListPattern string, forbidden []struct {

internal/core/accessmode/errors.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,3 +19,7 @@ var (
1919
ErrMultiUserUnknownRequiredLevel = errors.New("access.mode: multi_user unknown auth.required_level")
2020
ErrMultiUserInternalUnknownMode = errors.New("access.mode: internal error, unknown mode")
2121
)
22+
23+
// ErrMalformedListenAddress is returned by [ClassifyListenAddress] when the address is not
24+
// a usable host:port bind string (e.g. empty, missing port, or unparseable).
25+
var ErrMalformedListenAddress = errors.New("accessmode: malformed listen address")

internal/core/accessmode/listener.go

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,12 +29,14 @@ type ListenClassification struct {
2929
func ClassifyListenAddress(raw string) (ListenClassification, error) {
3030
s := strings.TrimSpace(raw)
3131
if s == "" {
32-
return ListenClassification{Raw: raw, Surface: SurfaceMalformed}, nil
32+
return ListenClassification{Raw: raw, Surface: SurfaceMalformed}, ErrMalformedListenAddress
3333
}
3434

3535
host, port, err := net.SplitHostPort(s)
3636
if err != nil {
37-
// Bare IP or hostname without a port (legacy configs); treat like host-only.
37+
// Bare IP or hostname without a port (legacy configs): classify surface with empty Port.
38+
// Other validation may require an explicit port (host:port) and map port-less values to
39+
// [SurfaceMalformed] with [ErrMalformedListenAddress] instead of this path.
3840
h := strings.TrimSpace(s)
3941
if ip := net.ParseIP(h); ip != nil {
4042
switch {
@@ -49,13 +51,14 @@ func ClassifyListenAddress(raw string) (ListenClassification, error) {
4951
if strings.EqualFold(h, "localhost") {
5052
return ListenClassification{Raw: raw, Host: h, Port: port, Surface: SurfaceLoopback}, nil
5153
}
52-
return ListenClassification{Raw: raw, Surface: SurfaceMalformed}, nil
54+
// Legacy bare host without port is supported above for IPs/localhost; other shapes are malformed.
55+
return ListenClassification{Raw: raw, Surface: SurfaceMalformed}, ErrMalformedListenAddress
5356
}
5457

5558
host = strings.TrimSpace(host)
5659
port = strings.TrimSpace(port)
5760
if port == "" {
58-
return ListenClassification{Raw: raw, Surface: SurfaceMalformed}, nil
61+
return ListenClassification{Raw: raw, Surface: SurfaceMalformed}, ErrMalformedListenAddress
5962
}
6063

6164
// All IPv4 interfaces or IPv6 unspecified.

internal/core/accessmode/listener_test.go

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package accessmode
22

33
import (
4+
"errors"
45
"testing"
56
)
67

@@ -25,8 +26,14 @@ func TestClassifyListenAddress(t *testing.T) {
2526
{raw: ":::9", want: SurfaceMalformed},
2627
} {
2728
got, err := ClassifyListenAddress(tc.raw)
28-
if err != nil && tc.want != SurfaceMalformed {
29-
t.Fatalf("raw %q: unexpected err %v", tc.raw, err)
29+
if tc.want == SurfaceMalformed {
30+
if !errors.Is(err, ErrMalformedListenAddress) {
31+
t.Fatalf("raw %q: want %v, got %v; surface %q", tc.raw, ErrMalformedListenAddress, err, got.Surface)
32+
}
33+
} else {
34+
if err != nil {
35+
t.Fatalf("raw %q: unexpected err %v", tc.raw, err)
36+
}
3037
}
3138
if got.Surface != tc.want {
3239
t.Fatalf("raw %q: want surface %q got %q (host=%q port=%q)", tc.raw, tc.want, got.Surface, got.Host, got.Port)

internal/core/accessmode/validate.go

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ package accessmode
33
import (
44
"fmt"
55
"strings"
6+
7+
sdkauth "github.com/matdev83/go-llm-interactive-proxy/pkg/lipsdk/auth"
68
)
79

810
// PostureInput carries effective access, listener, and auth policy for startup validation.
@@ -69,27 +71,27 @@ func validateMultiUserAuth(in PostureInput) error {
6971
if h == "" {
7072
return fmt.Errorf("%w", ErrMultiUserHandlerRequired)
7173
}
72-
if h == "local_noop" {
74+
if h == string(sdkauth.HandlerLocalNoop) {
7375
return fmt.Errorf(
7476
"%w (configure auth.handler local_api_key or remote)",
7577
ErrMultiUserLocalNoopDisallowed,
7678
)
7779
}
78-
if rl == "" || rl == "none" {
80+
if rl == "" || rl == string(sdkauth.LevelNone) {
7981
return fmt.Errorf(
8082
"%w (e.g. api_key or api_key_sso)",
8183
ErrMultiUserRequiredLevelTooWeak,
8284
)
8385
}
8486

8587
switch h {
86-
case "local_api_key", "remote":
88+
case string(sdkauth.HandlerLocalAPIKey), string(sdkauth.HandlerRemote):
8789
default:
8890
return fmt.Errorf("%w: %q", ErrMultiUserUnknownHandler, in.Handler)
8991
}
9092

9193
switch rl {
92-
case "api_key", "api_key_sso":
94+
case string(sdkauth.LevelAPIKey), string(sdkauth.LevelAPIKeySSO):
9395
default:
9496
return fmt.Errorf("%w: %q", ErrMultiUserUnknownRequiredLevel, in.RequiredLevel)
9597
}

internal/core/accessmode/validate_test.go

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package accessmode
22

33
import (
44
"errors"
5+
"sort"
56
"testing"
67
)
78

@@ -59,7 +60,7 @@ func TestValidatePosture_multiUserRequiresStrongAuth(t *testing.T) {
5960
}); err != nil {
6061
t.Fatal(err)
6162
}
62-
for name, tc := range map[string]struct {
63+
cases := map[string]struct {
6364
in PostureInput
6465
want error
6566
}{
@@ -91,9 +92,20 @@ func TestValidatePosture_multiUserRequiresStrongAuth(t *testing.T) {
9192
},
9293
want: ErrMultiUserIncompatibleNoAuth,
9394
},
94-
} {
95-
if err := ValidatePosture(tc.in); err == nil || !errors.Is(err, tc.want) {
96-
t.Fatalf("%s: want %v, got %v", name, tc.want, err)
97-
}
95+
}
96+
names := make([]string, 0, len(cases))
97+
for n := range cases {
98+
names = append(names, n)
99+
}
100+
sort.Strings(names)
101+
for _, name := range names {
102+
name := name
103+
tc := cases[name]
104+
t.Run(name, func(t *testing.T) {
105+
t.Parallel()
106+
if err := ValidatePosture(tc.in); err == nil || !errors.Is(err, tc.want) {
107+
t.Fatalf("want %v, got %v", tc.want, err)
108+
}
109+
})
98110
}
99111
}

internal/core/auth/errors.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,4 +3,8 @@ package auth
33
import "errors"
44

55
// ErrDuplicateLocalAPIKeyID is returned when two records share the same key_id.
6-
var ErrDuplicateLocalAPIKeyID = errors.New("auth.local_api_keys: duplicate key_id")
6+
var (
7+
ErrDuplicateLocalAPIKeyID = errors.New("auth.local_api_keys: duplicate key_id")
8+
ErrDuplicateLocalAPIKeyMaterial = errors.New("auth.local_api_keys: duplicate key material")
9+
ErrLocalAPIKeyEmpty = errors.New("auth.local_api_keys: key is required")
10+
)

0 commit comments

Comments
 (0)