Skip to content

Commit d80d09c

Browse files
fix(inspect): harden Docker discovery with batched inspect, fix find command, and add tests
- Fix P0 bug: `find` command passed `2>/dev/null` as literal arg and had ungrouped `-type f` that only applied to last `-name` pattern - Fix P0 perf: batch `docker inspect` to 2 execs instead of N+1 - Fix P1 security: add MaxComposeFileSize guard (10MB) to prevent OOM - Fix P1 correctness: ParseDockerSize now uses SI/decimal units matching Docker's actual format (1 GB = 1e9, not 1024^3) - Fix P1 style: remove emoji from all log messages in inspect package - Add deterministic output: sort networks, ports, volumes for stable diffs - Extract pure parsing functions for testability - Add 24 tests: unit (70%), integration (20%), e2e (10%) - Add constants for compose search paths, file names, sensitive keywords Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent b34aa76 commit d80d09c

50 files changed

Lines changed: 1810 additions & 463 deletions

Some content is hidden

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

.github/workflows/ci.yml

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,7 @@ PY
161161
run: scripts/ci/preflight.sh
162162

163163
- name: Install golangci-lint
164-
run: go install github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.0.0
164+
run: bash scripts/ci/install-golangci-lint.sh
165165

166166
- name: Run lint lane
167167
env:
@@ -245,6 +245,10 @@ PY
245245
run: |
246246
test -f outputs/ci/unit/report.json && cat outputs/ci/unit/report.json || true
247247
248+
- name: Alert on unit lane report
249+
if: always()
250+
run: python3 scripts/ci/report-alert.py ci-unit outputs/ci/unit/report.json
251+
248252
ci-deps-unit:
249253
name: ci-deps-unit
250254
runs-on: ubuntu-latest
@@ -295,6 +299,10 @@ PY
295299
run: |
296300
test -f outputs/ci/deps-unit/report.json && cat outputs/ci/deps-unit/report.json || true
297301
302+
- name: Alert on dependency-focused unit lane report
303+
if: always()
304+
run: python3 scripts/ci/report-alert.py ci-deps-unit outputs/ci/deps-unit/report.json
305+
298306
ci-integration:
299307
name: ci-integration
300308
runs-on: ubuntu-latest
@@ -344,6 +352,10 @@ PY
344352
run: |
345353
test -f outputs/ci/integration/report.json && cat outputs/ci/integration/report.json || true
346354
355+
- name: Alert on integration lane report
356+
if: always()
357+
run: python3 scripts/ci/report-alert.py ci-integration outputs/ci/integration/report.json
358+
347359
ci-e2e-smoke:
348360
name: ci-e2e-smoke
349361
runs-on: ubuntu-latest
@@ -391,6 +403,10 @@ PY
391403
run: |
392404
test -f outputs/ci/e2e-smoke/report.json && cat outputs/ci/e2e-smoke/report.json || true
393405
406+
- name: Alert on e2e smoke lane report
407+
if: always()
408+
run: python3 scripts/ci/report-alert.py ci-e2e-smoke outputs/ci/e2e-smoke/report.json
409+
394410
ci-fuzz:
395411
name: ci-fuzz
396412
runs-on: ubuntu-latest
@@ -438,6 +454,10 @@ PY
438454
run: |
439455
test -f outputs/ci/fuzz/report.json && cat outputs/ci/fuzz/report.json || true
440456
457+
- name: Alert on fuzz lane report
458+
if: always()
459+
run: python3 scripts/ci/report-alert.py ci-fuzz outputs/ci/fuzz/report.json
460+
441461
ci-e2e-full:
442462
name: ci-e2e-full
443463
if: github.event_name == 'schedule' || github.event_name == 'workflow_dispatch'
@@ -485,3 +505,7 @@ PY
485505
if: always()
486506
run: |
487507
test -f outputs/ci/e2e-full/report.json && cat outputs/ci/e2e-full/report.json || true
508+
509+
- name: Alert on e2e full lane report
510+
if: always()
511+
run: python3 scripts/ci/report-alert.py ci-e2e-full outputs/ci/e2e-full/report.json

.pre-commit-config.yaml

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -47,10 +47,10 @@ repos:
4747
# Local hooks for custom checks
4848
- repo: local
4949
hooks:
50-
# Enforce local/CI parity lane via mage-compatible entrypoint
50+
# Enforce local/CI parity lane via npm wrapper
5151
- id: ci-debug-parity
52-
name: CI debug parity gate (magew ci:debug)
53-
entry: ./magew ci:debug
52+
name: CI debug parity gate (npm run ci:debug)
53+
entry: npm run ci:debug --silent
5454
language: system
5555
pass_filenames: false
5656
require_serial: true
@@ -80,13 +80,13 @@ repos:
8080
pass_filenames: false
8181
description: Ensures code compiles successfully
8282

83-
# Verify E2E tests have build tags
84-
- id: verify-e2e-build-tags
85-
name: Verify E2E build tags
86-
entry: bash -c 'for f in test/e2e/*_test.go; do head -1 "$f" | grep -q "//go:build e2e" || { echo "ERROR: $f missing //go:build e2e tag"; exit 1; }; done'
83+
# Verify environment-dependent tests are build-tagged
84+
- id: verify-test-build-tags
85+
name: Verify test build tags
86+
entry: bash scripts/ci/check-test-tags.sh
8787
language: system
8888
pass_filenames: false
89-
description: Ensures all E2E tests have proper build tags
89+
description: Ensures environment-dependent tests keep their explicit build tags
9090

9191
# Check for deprecated benchmark pattern
9292
- id: check-benchmark-pattern

pkg/ai/ai.go

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"fmt"
1111
"io"
1212
"net/http"
13+
"net/url"
1314
"os"
1415
"strings"
1516
"time"
@@ -437,6 +438,10 @@ func (ai *AIAssistant) sendRequest(rc *eos_io.RuntimeContext, request AIRequest)
437438
url = ai.baseURL + "/messages"
438439
}
439440

441+
if err := validateRequestURL(url); err != nil {
442+
return nil, err
443+
}
444+
440445
// Create HTTP request
441446
req, err := http.NewRequestWithContext(rc.Ctx, "POST", url, bytes.NewBuffer(requestBody))
442447
if err != nil {
@@ -492,6 +497,23 @@ func (ai *AIAssistant) sendRequest(rc *eos_io.RuntimeContext, request AIRequest)
492497
return &aiResponse, nil
493498
}
494499

500+
func validateRequestURL(rawURL string) error {
501+
parsedURL, err := url.Parse(rawURL)
502+
if err != nil {
503+
return fmt.Errorf("invalid AI request URL: %w", err)
504+
}
505+
506+
if parsedURL.Scheme != "http" && parsedURL.Scheme != "https" {
507+
return fmt.Errorf("invalid AI request URL scheme %q", parsedURL.Scheme)
508+
}
509+
510+
if parsedURL.Host == "" {
511+
return fmt.Errorf("invalid AI request URL host")
512+
}
513+
514+
return nil
515+
}
516+
495517
// NewConversationContext creates a new conversation context
496518
func NewConversationContext(systemPrompt string) *ConversationContext {
497519
return &ConversationContext{

pkg/ai/ai_security_test.go

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -369,7 +369,10 @@ func TestAIErrorHandling(t *testing.T) {
369369
model: "claude-3-sonnet-20240229",
370370
maxTokens: 100,
371371
client: func() *httpclient.Client {
372-
c, _ := httpclient.NewClient(&httpclient.Config{Timeout: 30 * time.Second})
372+
cfg := httpclient.TestConfig()
373+
cfg.Timeout = 250 * time.Millisecond
374+
cfg.RetryConfig.MaxRetries = 0
375+
c, _ := httpclient.NewClient(cfg)
373376
return c
374377
}(),
375378
}
@@ -379,10 +382,8 @@ func TestAIErrorHandling(t *testing.T) {
379382
Messages: []AIMessage{},
380383
}
381384

382-
// This should handle the invalid URL gracefully
383385
_, err := assistant.Chat(rc, ctx, "test message")
384-
if err != nil {
385-
t.Logf("Expected error for invalid URL: %v", err)
386-
}
386+
assert.Error(t, err, "Should return error for invalid URL")
387+
assert.Contains(t, err.Error(), "invalid AI request URL")
387388
})
388389
}

pkg/ai/ai_test.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -334,8 +334,12 @@ func TestHTTPRequestSecurity(t *testing.T) {
334334
t.Run("request_timeout_security", func(t *testing.T) {
335335
// Create a server that delays response longer than client timeout
336336
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
337-
time.Sleep(10 * time.Second) // Delay much longer than client timeout
338-
w.WriteHeader(http.StatusOK)
337+
select {
338+
case <-time.After(10 * time.Second):
339+
w.WriteHeader(http.StatusOK)
340+
case <-r.Context().Done():
341+
return
342+
}
339343
}))
340344
defer server.Close()
341345

pkg/ai/comprehensive_test.go

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -480,7 +480,10 @@ func TestChatErrorHandling(t *testing.T) {
480480
model: "claude-3-sonnet-20240229",
481481
maxTokens: 100,
482482
client: func() *httpclient.Client {
483-
c, _ := httpclient.NewClient(&httpclient.Config{Timeout: 30 * time.Second})
483+
cfg := httpclient.TestConfig()
484+
cfg.Timeout = 250 * time.Millisecond
485+
cfg.RetryConfig.MaxRetries = 0
486+
c, _ := httpclient.NewClient(cfg)
484487
return c
485488
}(),
486489
}
@@ -490,11 +493,9 @@ func TestChatErrorHandling(t *testing.T) {
490493
Messages: []AIMessage{},
491494
}
492495

493-
// This should handle the invalid URL gracefully
494496
_, err := assistant.Chat(rc, ctx, "test message")
495-
if err != nil {
496-
t.Logf("Expected error for invalid URL: %v", err)
497-
}
497+
assert.Error(t, err, "Should return error for invalid URL")
498+
assert.Contains(t, err.Error(), "invalid AI request URL")
498499
})
499500
}
500501

pkg/authentication/comprehensive_test.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -567,10 +567,10 @@ func TestAuthenticationFlow(t *testing.T) {
567567
// TestTokenValidation tests token validation and lifecycle
568568
func TestTokenValidation(t *testing.T) {
569569
t.Parallel()
570-
mockProvider := new(MockAuthProvider)
571570

572571
t.Run("valid token", func(t *testing.T) {
573572
t.Parallel()
573+
mockProvider := new(MockAuthProvider)
574574
ctx := context.Background()
575575
token := generateTestToken()
576576

@@ -594,6 +594,7 @@ func TestTokenValidation(t *testing.T) {
594594

595595
t.Run("expired token", func(t *testing.T) {
596596
t.Parallel()
597+
mockProvider := new(MockAuthProvider)
597598
ctx := context.Background()
598599
token := generateTestToken()
599600

@@ -615,6 +616,7 @@ func TestTokenValidation(t *testing.T) {
615616

616617
t.Run("invalid token", func(t *testing.T) {
617618
t.Parallel()
619+
mockProvider := new(MockAuthProvider)
618620
ctx := context.Background()
619621
token := "invalid-token"
620622

@@ -628,6 +630,7 @@ func TestTokenValidation(t *testing.T) {
628630

629631
t.Run("revoked token", func(t *testing.T) {
630632
t.Parallel()
633+
mockProvider := new(MockAuthProvider)
631634
ctx := context.Background()
632635
token := generateTestToken()
633636

pkg/authentik/unified_client.go

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,15 @@ type UnifiedClient struct {
2929
httpClient *http.Client
3030
}
3131

32+
var waitForRetry = func(ctx context.Context, delay time.Duration) error {
33+
select {
34+
case <-time.After(delay):
35+
return nil
36+
case <-ctx.Done():
37+
return ctx.Err()
38+
}
39+
}
40+
3241
// NewUnifiedClient creates a new unified Authentik API client
3342
// SECURITY: Enforces TLS 1.2+ for all API communication
3443
// RELIABILITY: Includes retry logic with exponential backoff
@@ -69,18 +78,21 @@ func (c *UnifiedClient) DoRequest(ctx context.Context, method, path string, body
6978
var lastErr error
7079
maxRetries := 3
7180
baseDelay := time.Second
81+
nextDelay := time.Duration(0)
7282

7383
for attempt := 0; attempt <= maxRetries; attempt++ {
7484
if attempt > 0 {
7585
// P1 FIX: Use Retry-After header if present (from previous attempt)
7686
// RATIONALE: API knows best when to retry (rate limit windows, maintenance)
7787
// SECURITY: Prevents aggressive retry that could trigger IP ban
7888
// Note: retryAfter is set below when we get 429/503 response
79-
delay := baseDelay * time.Duration(1<<uint(attempt-1)) // Default: exponential backoff
80-
select {
81-
case <-time.After(delay):
82-
case <-ctx.Done():
83-
return nil, ctx.Err()
89+
delay := nextDelay
90+
if delay <= 0 {
91+
delay = baseDelay * time.Duration(1<<uint(attempt-1)) // Default: exponential backoff
92+
}
93+
nextDelay = 0
94+
if err := waitForRetry(ctx, delay); err != nil {
95+
return nil, err
8496
}
8597
}
8698

@@ -137,17 +149,12 @@ func (c *UnifiedClient) DoRequest(ctx context.Context, method, path string, body
137149
if retryAfter := resp.Header.Get("Retry-After"); retryAfter != "" {
138150
// Retry-After can be seconds (integer) or HTTP date
139151
if seconds, parseErr := strconv.Atoi(retryAfter); parseErr == nil && seconds > 0 {
140-
// Wait for specified seconds before next retry
141152
retryDelay := time.Duration(seconds) * time.Second
142153
// Cap at 5 minutes to prevent indefinite wait
143154
if retryDelay > 5*time.Minute {
144155
retryDelay = 5 * time.Minute
145156
}
146-
select {
147-
case <-time.After(retryDelay):
148-
case <-ctx.Done():
149-
return nil, ctx.Err()
150-
}
157+
nextDelay = retryDelay
151158
}
152159
// Note: HTTP date format parsing not implemented - use default backoff
153160
}

0 commit comments

Comments
 (0)