From be0c00f4cbd50461328629c54950bed3f2fa3056 Mon Sep 17 00:00:00 2001 From: Allisson Azevedo Date: Fri, 6 Mar 2026 10:58:56 -0300 Subject: [PATCH 1/2] feat(auth): Implement strict capability validation for policies Introduces centralized capability validation in the domain layer and enforces it within the CLI's policy parsing logic. This ensures that only predefined domain capabilities are accepted, preventing invalid configurations and aligning the CLI behavior with the documented security model. Key changes: - Added IsValidCapability and ValidCapabilities helpers to internal/auth/domain. - Updated ParseCapabilities in internal/ui to perform strict, case-sensitive validation against domain constants. - Expanded unit test coverage in internal/auth/domain and internal/ui to verify validation logic and error bubbling. - Updated technical documentation in conductor/tech-stack.md to reflect the new strict validation security measure. Closes: Validate Capabilities in ParseCapabilities track --- .../validate_capabilities_20260306/index.md | 5 +++ .../metadata.json | 8 ++++ .../validate_capabilities_20260306/plan.md | 26 ++++++++++++ .../validate_capabilities_20260306/spec.md | 30 +++++++++++++ conductor/tech-stack.md | 1 + conductor/tracks.md | 2 + internal/auth/domain/const.go | 24 +++++++++++ internal/auth/domain/const_test.go | 42 +++++++++++++++++++ internal/ui/policies.go | 13 ++++-- internal/ui/policies_test.go | 11 +++++ 10 files changed, 159 insertions(+), 3 deletions(-) create mode 100644 conductor/archive/validate_capabilities_20260306/index.md create mode 100644 conductor/archive/validate_capabilities_20260306/metadata.json create mode 100644 conductor/archive/validate_capabilities_20260306/plan.md create mode 100644 conductor/archive/validate_capabilities_20260306/spec.md create mode 100644 internal/auth/domain/const_test.go diff --git a/conductor/archive/validate_capabilities_20260306/index.md b/conductor/archive/validate_capabilities_20260306/index.md new file mode 100644 index 0000000..c7bb72d --- /dev/null +++ b/conductor/archive/validate_capabilities_20260306/index.md @@ -0,0 +1,5 @@ +# Track validate_capabilities_20260306 Context + +- [Specification](./spec.md) +- [Implementation Plan](./plan.md) +- [Metadata](./metadata.json) diff --git a/conductor/archive/validate_capabilities_20260306/metadata.json b/conductor/archive/validate_capabilities_20260306/metadata.json new file mode 100644 index 0000000..fb5a80a --- /dev/null +++ b/conductor/archive/validate_capabilities_20260306/metadata.json @@ -0,0 +1,8 @@ +{ + "track_id": "validate_capabilities_20260306", + "type": "other", + "status": "new", + "created_at": "2026-03-06T10:00:00Z", + "updated_at": "2026-03-06T10:00:00Z", + "description": "Validate Capabilities in ParseCapabilities" +} diff --git a/conductor/archive/validate_capabilities_20260306/plan.md b/conductor/archive/validate_capabilities_20260306/plan.md new file mode 100644 index 0000000..21d39b7 --- /dev/null +++ b/conductor/archive/validate_capabilities_20260306/plan.md @@ -0,0 +1,26 @@ +# Implementation Plan: Validate Capabilities in ParseCapabilities + +## Overview +Enhance `ParseCapabilities` in `internal/ui` to strictly validate capabilities against the `internal/auth/domain` constants. + +## Phase 1: Domain Enhancement [checkpoint: 39909a2] +Add validation helpers to the domain package to ensure extensibility. + +- [x] Task: Add `ValidCapabilities()` and `IsValidCapability()` to `internal/auth/domain/const.go`. 6d24b0e +- [x] Task: Add unit tests for `IsValidCapability()` in `internal/auth/domain/const_test.go` (create file if needed). 6d24b0e +- [x] Task: Conductor - User Manual Verification 'Phase 1: Domain Enhancement' (Protocol in workflow.md) + +## Phase 2: UI Implementation [checkpoint: 6ce42dd] +Update `ParseCapabilities` to use the domain validation. + +- [x] Task: Update `internal/ui/policies_test.go` with failing test cases for invalid capabilities and case-sensitivity. d7770f6 +- [x] Task: Update `internal/ui/policies.go`'s `ParseCapabilities` function to perform validation using the domain helpers. d7770f6 +- [x] Task: Verify that `PromptForPolicies` correctly bubbles up these errors (existing tests or add new ones). d7770f6 +- [x] Task: Conductor - User Manual Verification 'Phase 2: UI Implementation' (Protocol in workflow.md) + +## Phase 3: Documentation and Quality Gates [checkpoint: 911c90f] +Ensure everything is consistent and meets quality standards. + +- [x] Task: Verify `docs/auth/policies.md` and `docs/cli-commands.md` align with the strict validation (already appear to, but double check examples). 3bb2424 +- [x] Task: Run full test suite (`make test`) and linting (`make lint`). 3bb2424 +- [x] Task: Conductor - User Manual Verification 'Phase 3: Documentation and Quality Gates' (Protocol in workflow.md) diff --git a/conductor/archive/validate_capabilities_20260306/spec.md b/conductor/archive/validate_capabilities_20260306/spec.md new file mode 100644 index 0000000..2cbf19a --- /dev/null +++ b/conductor/archive/validate_capabilities_20260306/spec.md @@ -0,0 +1,30 @@ +# Specification: Validate Capabilities in ParseCapabilities + +## Overview +Currently, `ParseCapabilities` in the `internal/ui` package accepts any non-empty string as a valid capability. This track implements strict validation to ensure only predefined domain capabilities are accepted, aligning the CLI behavior with the documented security model. + +## Functional Requirements +- **Strict Validation:** Each capability in the input string must match one of the valid capabilities defined in the `internal/auth/domain` package. +- **Error Handling:** If any single capability is invalid or unknown, the entire parsing operation must fail with a descriptive error. +- **Normalization:** Matching must be case-sensitive. Only exact, lowercase matches (as defined in the domain) are considered valid. +- **Extensible Implementation:** + - Add a `IsValidCapability(Capability) bool` or `ValidCapabilities() []Capability` helper to `internal/auth/domain`. + - `ParseCapabilities` should use this helper to perform validation. +- **Documentation Alignment:** Ensure that `docs/auth/policies.md` and any other relevant documentation accurately reflect these requirements (already appears aligned, but requires final verification). + +## Domain Capabilities (as of v1.0.0) +- `read` +- `write` +- `delete` +- `encrypt` +- `decrypt` +- `rotate` + +## Acceptance Criteria +- `ParseCapabilities("read,write")` succeeds and returns `[]authDomain.Capability{"read", "write"}`. +- `ParseCapabilities("read, invalid")` fails with an error indicating that "invalid" is not a valid capability. +- `ParseCapabilities("READ")` fails (strict case-sensitivity). +- `ParseCapabilities("")` and `ParseCapabilities(" , ")` continue to fail. +- Unit tests in `internal/ui/policies_test.go` are updated/added. +- New unit tests for domain helper are added. +- `PromptForPolicies` flow correctly handles these validation errors. diff --git a/conductor/tech-stack.md b/conductor/tech-stack.md index 8060112..29392be 100644 --- a/conductor/tech-stack.md +++ b/conductor/tech-stack.md @@ -15,6 +15,7 @@ - **Password Hashing:** [go-pwdhash](https://github.com/allisson/go-pwdhash) - Argon2id hashing for secure storage of client secrets and passwords. - **Request Body Size Limiting:** Middleware to prevent DoS attacks from large payloads. - **Secret Value Size Limiting:** Global limit on individual secret values to ensure predictable storage and memory usage. +- **Strict Capability Validation:** Centralized domain helpers for validating policy capabilities (`read`, `write`, `delete`, `encrypt`, `decrypt`, `rotate`) in CLI and API layers. - **Secret Path Validation:** Strict naming rules for secret paths (alphanumeric, -, _, /) to ensure consistency and security. - **Audit Signing:** HMAC-SHA256 for tamper-evident cryptographic audit logs. diff --git a/conductor/tracks.md b/conductor/tracks.md index 22d3d64..0b5c54e 100644 --- a/conductor/tracks.md +++ b/conductor/tracks.md @@ -1,3 +1,5 @@ # Project Tracks This file tracks all major tracks for the project. Each track has its own detailed plan in its respective folder. + +--- diff --git a/internal/auth/domain/const.go b/internal/auth/domain/const.go index 65cbb7b..ddc3b13 100644 --- a/internal/auth/domain/const.go +++ b/internal/auth/domain/const.go @@ -25,3 +25,27 @@ const ( // RotateCapability allows rotating cryptographic keys. RotateCapability Capability = "rotate" ) + +// ValidCapabilities returns a list of all defined capabilities in the system. +// Used for validation and UI generation. +func ValidCapabilities() []Capability { + return []Capability{ + ReadCapability, + WriteCapability, + DeleteCapability, + EncryptCapability, + DecryptCapability, + RotateCapability, + } +} + +// IsValidCapability checks if a capability is valid and exists in the system. +// Used for strict validation in policy parsing. +func IsValidCapability(cap Capability) bool { + for _, valid := range ValidCapabilities() { + if cap == valid { + return true + } + } + return false +} diff --git a/internal/auth/domain/const_test.go b/internal/auth/domain/const_test.go new file mode 100644 index 0000000..a083ab1 --- /dev/null +++ b/internal/auth/domain/const_test.go @@ -0,0 +1,42 @@ +package domain + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestValidCapabilities(t *testing.T) { + caps := ValidCapabilities() + assert.Len(t, caps, 6) + assert.Contains(t, caps, ReadCapability) + assert.Contains(t, caps, WriteCapability) + assert.Contains(t, caps, DeleteCapability) + assert.Contains(t, caps, EncryptCapability) + assert.Contains(t, caps, DecryptCapability) + assert.Contains(t, caps, RotateCapability) +} + +func TestIsValidCapability(t *testing.T) { + tests := []struct { + name string + cap Capability + expected bool + }{ + {"read is valid", ReadCapability, true}, + {"write is valid", WriteCapability, true}, + {"delete is valid", DeleteCapability, true}, + {"encrypt is valid", EncryptCapability, true}, + {"decrypt is valid", DecryptCapability, true}, + {"rotate is valid", RotateCapability, true}, + {"invalid is not valid", Capability("invalid"), false}, + {"empty is not valid", Capability(""), false}, + {"case sensitive is not valid", Capability("READ"), false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.expected, IsValidCapability(tt.cap)) + }) + } +} diff --git a/internal/ui/policies.go b/internal/ui/policies.go index dd6f975..087cacd 100644 --- a/internal/ui/policies.go +++ b/internal/ui/policies.go @@ -104,15 +104,22 @@ func PromptForPoliciesUpdate( } // ParseCapabilities converts a comma-separated string into a slice of Capability. +// Performs strict validation against valid domain capabilities. func ParseCapabilities(input string) ([]authDomain.Capability, error) { parts := strings.Split(input, ",") capabilities := make([]authDomain.Capability, 0, len(parts)) for _, part := range parts { - cap := authDomain.Capability(strings.TrimSpace(part)) - if cap != "" { - capabilities = append(capabilities, cap) + trimmed := strings.TrimSpace(part) + if trimmed == "" { + continue } + + cap := authDomain.Capability(trimmed) + if !authDomain.IsValidCapability(cap) { + return nil, fmt.Errorf("invalid capability: '%s'", trimmed) + } + capabilities = append(capabilities, cap) } if len(capabilities) == 0 { diff --git a/internal/ui/policies_test.go b/internal/ui/policies_test.go index 9773e32..1e9b6ad 100644 --- a/internal/ui/policies_test.go +++ b/internal/ui/policies_test.go @@ -18,6 +18,8 @@ func TestParseCapabilities(t *testing.T) { }{ {"read,write", []authDomain.Capability{"read", "write"}, false}, {"read , write ", []authDomain.Capability{"read", "write"}, false}, + {"read,invalid", nil, true}, + {"READ", nil, true}, {"", nil, true}, {" , ", nil, true}, } @@ -66,4 +68,13 @@ func TestPromptForPolicies(t *testing.T) { require.Error(t, err) require.Contains(t, err.Error(), "path cannot be empty") }) + + t.Run("invalid-capability", func(t *testing.T) { + input := "secret/*\ninvalid\n" + var output bytes.Buffer + _, err := PromptForPolicies(strings.NewReader(input), &output) + + require.Error(t, err) + require.Contains(t, err.Error(), "invalid capability: 'invalid'") + }) } From 708647896cd5c85f595a2b5dbed7e19167115761 Mon Sep 17 00:00:00 2001 From: Allisson Azevedo Date: Fri, 6 Mar 2026 11:13:11 -0300 Subject: [PATCH 2/2] fix gosec issues --- cmd/app/commands/create_client.go | 1 + internal/auth/http/token_handler_test.go | 1 + internal/tokenization/service/luhn_generator.go | 1 + 3 files changed, 3 insertions(+) diff --git a/cmd/app/commands/create_client.go b/cmd/app/commands/create_client.go index 0e48b0a..715e9dd 100644 --- a/cmd/app/commands/create_client.go +++ b/cmd/app/commands/create_client.go @@ -31,6 +31,7 @@ func (r *CreateClientResult) ToText() string { // ToJSON returns a JSON representation of the creation result. func (r *CreateClientResult) ToJSON() string { + // #nosec G117 - Intentionally marshaling secret for CLI output. This is shown once during client creation. jsonBytes, _ := json.MarshalIndent(r, "", " ") return string(jsonBytes) } diff --git a/internal/auth/http/token_handler_test.go b/internal/auth/http/token_handler_test.go index 00fd1a7..1574ce6 100644 --- a/internal/auth/http/token_handler_test.go +++ b/internal/auth/http/token_handler_test.go @@ -55,6 +55,7 @@ func TestTokenHandler_IssueTokenHandler(t *testing.T) { w := httptest.NewRecorder() c, _ := gin.CreateTestContext(w) + // #nosec G117 - Test code with hardcoded test fixtures, not real secrets body, _ := json.Marshal(request) c.Request, _ = http.NewRequest(http.MethodPost, "/v1/token", bytes.NewBuffer(body)) diff --git a/internal/tokenization/service/luhn_generator.go b/internal/tokenization/service/luhn_generator.go index 561e6b5..8e8bf1e 100644 --- a/internal/tokenization/service/luhn_generator.go +++ b/internal/tokenization/service/luhn_generator.go @@ -43,6 +43,7 @@ func (g *luhnGenerator) Generate(length int) (string, error) { // Convert to string token := make([]byte, length) for i, d := range digits { + // #nosec G115 - Safe conversion: d is guaranteed to be 0-9, so '0'+d produces ASCII 48-57 (well within byte range) token[i] = byte('0' + d) }