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

Add secret.Warn() calls for all silent anomalous conditions #20

Merged
sneak merged 1 commits from clawbot/secret:audit/add-warnings into main 2026-02-20 09:22:29 +01:00
Conversation 1 Commits 1 Files Changed 7 +40 -5
clawbot commented 2026-02-20 09:04:01 +01:00
Collaborator
Copy Link

Closes #19

Audit of the codebase found 9 locations where errors or anomalous conditions were silently swallowed or only logged via Debug(). Users should be informed when something unexpected happens.

Changes

File Change
internal/secret/helpers.go Warn on config dir fallback to ~/.config
internal/cli/info_helper.go Warn when vault/secret stats cannot be read
internal/cli/unlockers.go Warn on metadata read/parse failures in UnlockersList (fixes FIXMEs)
internal/cli/unlockers.go Warn on fallback ID generation
internal/cli/unlockers.go Warn on errors during duplicate checking in checkUnlockerExists
internal/cli/completions.go Warn on unlocker metadata read/parse failures
internal/cli/version.go Upgrade metadata load failure from Debug to Warn
internal/cli/secrets.go Upgrade file close failure from Debug to Warn
internal/secret/version.go Warn on malformed version directory names

make check results

golangci-lint: 0 issues
go vet: pass
go test: all pass
Closes #19 Audit of the codebase found 9 locations where errors or anomalous conditions were silently swallowed or only logged via `Debug()`. Users should be informed when something unexpected happens. ## Changes | File | Change | |------|--------| | `internal/secret/helpers.go` | Warn on config dir fallback to `~/.config` | | `internal/cli/info_helper.go` | Warn when vault/secret stats cannot be read | | `internal/cli/unlockers.go` | Warn on metadata read/parse failures in `UnlockersList` (fixes FIXMEs) | | `internal/cli/unlockers.go` | Warn on fallback ID generation | | `internal/cli/unlockers.go` | Warn on errors during duplicate checking in `checkUnlockerExists` | | `internal/cli/completions.go` | Warn on unlocker metadata read/parse failures | | `internal/cli/version.go` | Upgrade metadata load failure from `Debug` to `Warn` | | `internal/cli/secrets.go` | Upgrade file close failure from `Debug` to `Warn` | | `internal/secret/version.go` | Warn on malformed version directory names | ## `make check` results ``` golangci-lint: 0 issues go vet: pass go test: all pass ```
clawbot added 1 commit 2026-02-20 09:04:02 +01:00 
Audit of the codebase found 9 locations where errors or anomalous
conditions were silently swallowed or only logged via Debug(). Users
should be informed when something unexpected happens, even if the
program can continue.

Changes:
- DetermineStateDir: warn on config dir fallback to ~/.config
- info_helper: warn when vault/secret stats cannot be read
- unlockers list: warn on metadata read/parse failures (fixes FIXMEs)
- unlockers list: warn on fallback ID generation
- checkUnlockerExists: warn on errors during duplicate checking
- completions: warn on unlocker metadata read/parse failures
- version list: upgrade metadata load failure from Debug to Warn
- secrets: upgrade file close failure from Debug to Warn
- version naming: warn on malformed version directory names

Closes #19
clawbot commented 2026-02-20 09:04:09 +01:00
Author
Collaborator
Copy Link

make check results

golangci-lint run --timeout 5m
0 issues.

go vet ./...
(pass, only cgo deprecation warnings from upstream dependency)

go test ./...
ok  git.eeqj.de/sneak/secret/internal/cli      5.928s
ok  git.eeqj.de/sneak/secret/internal/secret   3.175s
ok  git.eeqj.de/sneak/secret/internal/vault    1.642s
ok  git.eeqj.de/sneak/secret/pkg/agehd         0.673s
ok  git.eeqj.de/sneak/secret/pkg/bip85         0.808s

All checks pass. ✅

## `make check` results ``` golangci-lint run --timeout 5m 0 issues. go vet ./... (pass, only cgo deprecation warnings from upstream dependency) go test ./... ok git.eeqj.de/sneak/secret/internal/cli 5.928s ok git.eeqj.de/sneak/secret/internal/secret 3.175s ok git.eeqj.de/sneak/secret/internal/vault 1.642s ok git.eeqj.de/sneak/secret/pkg/agehd 0.673s ok git.eeqj.de/sneak/secret/pkg/bip85 0.808s ``` All checks pass. ✅
clawbot reviewed 2026-02-20 09:15:04 +01:00
clawbot left a comment
Author
Collaborator
Copy Link

Code Review: Add secret.Warn() calls

Overall: Clean ✅

This PR replaces silent continue statements and secret.Debug() calls with secret.Warn() throughout the codebase. Good improvement for observability.

Changes

  • completions.go: Adds warnings when reading unlocker directories or metadata fails during tab completion. Reasonable — completion failures are usually silent and confusing.
  • info_helper.go: Warns on failed vault/secret/version directory reads. Adds import for secret package.
  • secrets.go: Upgrades file close error from Debug to Warn. Appropriate — file close failures can indicate data loss.
  • unlockers.go: Multiple locations where errors were silently swallowed (some with // FIXME comments) now log warnings. The FIXME comments are removed — good cleanup.
  • version.go: Metadata load failure upgraded from Debug to Warn.
  • helpers.go: Config dir fallback now warns. Good — users should know when falling back to default paths.
  • version.go (internal/secret): Malformed version directory names now warned about.

Observations

  • All warn calls include structured key-value pairs ("error", "path", etc.) — consistent with slog patterns.
  • No behavioral changes — all continue paths preserved. This is purely additive logging.
  • Blank lines after some warn calls are a bit inconsistent but harmless.

No issues found. Ready for make check verification.

## Code Review: Add secret.Warn() calls **Overall: Clean** ✅ This PR replaces silent `continue` statements and `secret.Debug()` calls with `secret.Warn()` throughout the codebase. Good improvement for observability. ### Changes - **completions.go**: Adds warnings when reading unlocker directories or metadata fails during tab completion. Reasonable — completion failures are usually silent and confusing. - **info_helper.go**: Warns on failed vault/secret/version directory reads. Adds import for `secret` package. - **secrets.go**: Upgrades file close error from Debug to Warn. Appropriate — file close failures can indicate data loss. - **unlockers.go**: Multiple locations where errors were silently swallowed (some with `// FIXME` comments) now log warnings. The FIXME comments are removed — good cleanup. - **version.go**: Metadata load failure upgraded from Debug to Warn. - **helpers.go**: Config dir fallback now warns. Good — users should know when falling back to default paths. - **version.go (internal/secret)**: Malformed version directory names now warned about. ### Observations - All warn calls include structured key-value pairs (`"error"`, `"path"`, etc.) — consistent with slog patterns. - No behavioral changes — all `continue` paths preserved. This is purely additive logging. - Blank lines after some warn calls are a bit inconsistent but harmless. No issues found. Ready for `make check` verification.
sneak merged commit 797d2678c8 into main 2026-02-20 09:22:29 +01:00
clawbot self-assigned this 2026-02-20 09:23:56 +01:00
clawbot added the
needs-checks
 label 2026-02-20 09:23:57 +01:00
Sign in to join this conversation.
Reviewers
No reviewers
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
clawbot
1 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#20
No description provided.
Delete Branch "clawbot/secret:audit/add-warnings"

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?