sneak/secret
1
0
Fork 1
Code Issues 1 Pull Requests 2 Actions Packages Projects Releases Wiki Activity

Allow uppercase letters in secret names (closes #2) #16

Merged
sneak merged 4 commits from clawbot/secret:fix/issue-2 into main 2026-02-20 08:57:20 +01:00
Conversation 2 Commits 4 Files Changed 4 +50 -6
clawbot commented 2026-02-15 23:04:16 +01:00
Collaborator
Copy Link

Summary

Fixes #2 — isValidSecretName() regex rejected uppercase letters.

Changes

  • Changed regex from ^[a-z0-9\.\-\_\/]+$ to ^[a-zA-Z0-9\.\-\_\/]+$ in internal/vault/secrets.go
  • Updated test helper in internal/secret/secret_test.go to allow uppercase
  • Added internal/vault/secrets_name_test.go with comprehensive uppercase test cases

Test Output

Vault package tests

=== RUN   TestIsValidSecretNameUppercase
=== RUN   TestIsValidSecretNameUppercase/valid-name
=== RUN   TestIsValidSecretNameUppercase/valid.name
=== RUN   TestIsValidSecretNameUppercase/valid_name
=== RUN   TestIsValidSecretNameUppercase/valid/path/name
=== RUN   TestIsValidSecretNameUppercase/123valid
=== RUN   TestIsValidSecretNameUppercase/Valid-Upper-Name
=== RUN   TestIsValidSecretNameUppercase/2025-11-21-ber1app1-vaultik-test-bucket-AKI
=== RUN   TestIsValidSecretNameUppercase/MixedCase/Path/Name
=== RUN   TestIsValidSecretNameUppercase/ALLUPPERCASE
=== RUN   TestIsValidSecretNameUppercase/ABC123
=== RUN   TestIsValidSecretNameUppercase/#00
=== RUN   TestIsValidSecretNameUppercase/invalid_name
=== RUN   TestIsValidSecretNameUppercase/invalid@name
=== RUN   TestIsValidSecretNameUppercase/.dotstart
=== RUN   TestIsValidSecretNameUppercase//leading-slash
=== RUN   TestIsValidSecretNameUppercase/trailing-slash/
=== RUN   TestIsValidSecretNameUppercase/double//slash
--- PASS: TestIsValidSecretNameUppercase (0.00s)
PASS
ok  git.eeqj.de/sneak/secret/internal/vault 0.182s

Secret package tests

=== RUN   TestSecretNameValidation
=== RUN   TestSecretNameValidation/valid-name
=== RUN   TestSecretNameValidation/valid.name
=== RUN   TestSecretNameValidation/valid_name
=== RUN   TestSecretNameValidation/valid/path/name
=== RUN   TestSecretNameValidation/123valid
=== RUN   TestSecretNameValidation/#00
=== RUN   TestSecretNameValidation/Valid-Upper-Name
=== RUN   TestSecretNameValidation/2025-11-21-ber1app1-vaultik-test-bucket-AKI
=== RUN   TestSecretNameValidation/MixedCase/Path/Name
=== RUN   TestSecretNameValidation/invalid_name
=== RUN   TestSecretNameValidation/invalid@name
--- PASS: TestSecretNameValidation (0.00s)
PASS
ok  git.eeqj.de/sneak/secret/internal/secret 0.180s

Linting

go vet passes. golangci-lint not available in environment; only cgo deprecation warnings from keychain dependency (unrelated).

## Summary Fixes #2 — `isValidSecretName()` regex rejected uppercase letters. ## Changes - Changed regex from `^[a-z0-9\.\-\_\/]+$` to `^[a-zA-Z0-9\.\-\_\/]+$` in `internal/vault/secrets.go` - Updated test helper in `internal/secret/secret_test.go` to allow uppercase - Added `internal/vault/secrets_name_test.go` with comprehensive uppercase test cases ## Test Output ### Vault package tests ``` === RUN TestIsValidSecretNameUppercase === RUN TestIsValidSecretNameUppercase/valid-name === RUN TestIsValidSecretNameUppercase/valid.name === RUN TestIsValidSecretNameUppercase/valid_name === RUN TestIsValidSecretNameUppercase/valid/path/name === RUN TestIsValidSecretNameUppercase/123valid === RUN TestIsValidSecretNameUppercase/Valid-Upper-Name === RUN TestIsValidSecretNameUppercase/2025-11-21-ber1app1-vaultik-test-bucket-AKI === RUN TestIsValidSecretNameUppercase/MixedCase/Path/Name === RUN TestIsValidSecretNameUppercase/ALLUPPERCASE === RUN TestIsValidSecretNameUppercase/ABC123 === RUN TestIsValidSecretNameUppercase/#00 === RUN TestIsValidSecretNameUppercase/invalid_name === RUN TestIsValidSecretNameUppercase/invalid@name === RUN TestIsValidSecretNameUppercase/.dotstart === RUN TestIsValidSecretNameUppercase//leading-slash === RUN TestIsValidSecretNameUppercase/trailing-slash/ === RUN TestIsValidSecretNameUppercase/double//slash --- PASS: TestIsValidSecretNameUppercase (0.00s) PASS ok git.eeqj.de/sneak/secret/internal/vault 0.182s ``` ### Secret package tests ``` === RUN TestSecretNameValidation === RUN TestSecretNameValidation/valid-name === RUN TestSecretNameValidation/valid.name === RUN TestSecretNameValidation/valid_name === RUN TestSecretNameValidation/valid/path/name === RUN TestSecretNameValidation/123valid === RUN TestSecretNameValidation/#00 === RUN TestSecretNameValidation/Valid-Upper-Name === RUN TestSecretNameValidation/2025-11-21-ber1app1-vaultik-test-bucket-AKI === RUN TestSecretNameValidation/MixedCase/Path/Name === RUN TestSecretNameValidation/invalid_name === RUN TestSecretNameValidation/invalid@name --- PASS: TestSecretNameValidation (0.00s) PASS ok git.eeqj.de/sneak/secret/internal/secret 0.180s ``` ## Linting `go vet` passes. `golangci-lint` not available in environment; only cgo deprecation warnings from keychain dependency (unrelated).
clawbot added 1 commit 2026-02-15 23:04:17 +01:00 
The isValidSecretName() regex only allowed lowercase letters [a-z], rejecting
valid secret names containing uppercase characters (e.g. AWS access key IDs).

Changed regex from ^[a-z0-9\.\-\_\/]+$ to ^[a-zA-Z0-9\.\-\_\/]+$ and added
tests for uppercase secret names in both vault and secret packages.
clawbot reviewed 2026-02-15 23:16:10 +01:00
clawbot left a comment
Author
Collaborator
Copy Link

Self-Review: MERGE-READY ✅

Regex change ([a-z0-9] → [a-zA-Z0-9]): Correct and minimal. Both the vault package regex and the test helper character-range check are updated consistently.

Tests: Comprehensive. New secrets_name_test.go covers uppercase, mixed case, all-uppercase, and uppercase with paths. Existing invalid cases (spaces, @, leading/trailing/double slashes, dot-start, empty string) are preserved. The secret_test.go helper is updated in parallel.

Edge cases:

  • Unicode/accented chars: Already rejected by the allowlist regex — no regression.
  • Special chars (!, #, $, etc.): Already rejected — no regression.
  • The change is purely additive (widens a-z to a-zA-Z), so no existing valid names break.

Code style: Consistent with existing patterns. Comment updated to match. New test file follows the same table-driven style.

Verdict: Straightforward, well-tested, no concerns. Ready for maintainer merge.

## Self-Review: MERGE-READY ✅ **Regex change** (`[a-z0-9]` → `[a-zA-Z0-9]`): Correct and minimal. Both the vault package regex and the test helper character-range check are updated consistently. **Tests**: Comprehensive. New `secrets_name_test.go` covers uppercase, mixed case, all-uppercase, and uppercase with paths. Existing invalid cases (spaces, `@`, leading/trailing/double slashes, dot-start, empty string) are preserved. The secret_test.go helper is updated in parallel. **Edge cases**: - Unicode/accented chars: Already rejected by the allowlist regex — no regression. - Special chars (`!`, `#`, `$`, etc.): Already rejected — no regression. - The change is purely additive (widens `a-z` to `a-zA-Z`), so no existing valid names break. **Code style**: Consistent with existing patterns. Comment updated to match. New test file follows the same table-driven style. **Verdict**: Straightforward, well-tested, no concerns. Ready for maintainer merge.
clawbot commented 2026-02-19 06:39:29 +01:00
Author
Collaborator
Copy Link

Code Review

Verdict: LGTM ✅

Simple regex change from [a-z0-9\.\-\_\/] to [a-zA-Z0-9\.\-\_\/] to allow uppercase letters in secret names. Motivated by real-world AWS key IDs containing uppercase.

Changes are consistent across both internal/vault/secrets.go (the real validator) and internal/secret/secret_test.go (the test helper's copy). New secrets_name_test.go covers uppercase variants.

Merge order: Should be merged after #15 (path traversal fix) since both touch isValidSecretName().

## Code Review **Verdict: LGTM ✅** Simple regex change from `[a-z0-9\.\-\_\/]` to `[a-zA-Z0-9\.\-\_\/]` to allow uppercase letters in secret names. Motivated by real-world AWS key IDs containing uppercase. Changes are consistent across both `internal/vault/secrets.go` (the real validator) and `internal/secret/secret_test.go` (the test helper's copy). New `secrets_name_test.go` covers uppercase variants. **Merge order:** Should be merged after #15 (path traversal fix) since both touch `isValidSecretName()`.
clawbot added 1 commit 2026-02-20 08:41:45 +01:00
clawbot added 1 commit 2026-02-20 08:42:41 +01:00
clawbot commented 2026-02-20 08:45:20 +01:00
Author
Collaborator
Copy Link

✅ make check passes cleanly (0 lint issues, all tests pass). Rebased on main. Fixed: gosec G204 suppression (pre-existing), updated integration test to accept uppercase secret names (was incorrectly rejecting them).

✅ `make check` passes cleanly (0 lint issues, all tests pass). Rebased on main. Fixed: gosec G204 suppression (pre-existing), updated integration test to accept uppercase secret names (was incorrectly rejecting them).
clawbot requested review from sneak 2026-02-20 08:45:50 +01:00
sneak added 1 commit 2026-02-20 08:57:06 +01:00
sneak merged commit 09be20a044 into main 2026-02-20 08:57:20 +01:00
Sign in to join this conversation.
Reviewers
No reviewers
sneak
Labels
Clear labels   
merge-ready

   
merge-ready

   
needs-checks

   
needs-checks

   
needs-rebase

   
needs-rebase

   
needs-review

   
needs-review

   
needs-rework

   
needs-rework
No Label
merge-ready
merge-ready
needs-checks
needs-checks
needs-rebase
needs-rebase
needs-review
needs-review
needs-rework
needs-rework
Milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: sneak/secret#16
No description provided.
Delete Branch "clawbot/secret:fix/issue-2"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?