Skip to content

Commit 17da975

Browse files
danieljustusdanieljustus
authored
Session PR: Security fixes + config rename + cmd alias cleanup (#197)
* Restrict agent-controlled output_path to prevent arbitrary filesystem writes Validate output_path in handleGenerateTemplate using filepath.Rel against the vault directory. Rejects paths that escape via ../, absolute paths outside the vault, or any traversal attempt. Closes #193 * Validate API template base_url against private/internal IP ranges to prevent SSRF Add parse-time host validation in apitemplates.parseTemplate that checks base_url against loopback, link-local, and RFC 1918 private ranges. Rejects templates pointing at internal addresses unless allow_private: true is set. Closes #194 * fix(config): rename envWhitelist to envAllowlist with backward-compatible migration - Rename EnvWhitelist struct field to EnvAllowlist with new yaml tag envAllowlist - Keep EnvWhitelist as deprecated alias for backward compatibility - Add deprecation warning when envWhitelist is detected in config - Update merge logic, config keys, tests, and documentation - Add tests for both new key and backward compatibility Closes #195 * refactor(cmd): remove re-export aliases from cmd/root.go Remove 25+ package-level variable aliases (var X = cli.X) from cmd/root.go. All cmd/*.go files now import internal/cli directly and use cli.X instead of the shim aliases. This completes the migration that split CLI logic into internal/cli. cmd/root.go now only contains Execute(), SetVersionInfo(), AppVersion(), and quiet-mode print helpers. Closes #196 * Apply gofmt formatting fix Fixes formatting in internal/config/config.go. Refs PR #197
1 parent 5ac1854 commit 17da975

29 files changed

Lines changed: 476 additions & 103 deletions

‎cmd/cmd_reg_test.go‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -255,7 +255,7 @@ func TestUnlockVaultWithEnvVar(t *testing.T) {
255255
vaultFlag.Changed = false
256256
}
257257

258-
_, err := unlockVault(vaultDir, false)
258+
_, err := cli.UnlockVault(vaultDir, false)
259259
if err != nil {
260260
t.Errorf("unlockVault with env var failed: %v", err)
261261
}
@@ -285,7 +285,7 @@ func TestUnlockVaultNoPassphrase(t *testing.T) {
285285
vaultFlag.Changed = false
286286
}
287287

288-
_, err := unlockVault(vaultDir, false)
288+
_, err := cli.UnlockVault(vaultDir, false)
289289
if err == nil {
290290
t.Error("expected error when no passphrase available")
291291
}
@@ -317,7 +317,7 @@ func TestUnlockVaultWrongPassphrase(t *testing.T) {
317317
vaultFlag.Changed = false
318318
}
319319

320-
_, err := unlockVault(vaultDir, false)
320+
_, err := cli.UnlockVault(vaultDir, false)
321321
if err == nil {
322322
t.Error("expected error for wrong passphrase")
323323
}
@@ -336,12 +336,12 @@ func TestVaultPathWithEnvVarOpenPassVault(t *testing.T) {
336336
defer func() { vault = origVault }()
337337
vault = "~/should-not-be-used"
338338

339-
path, err := vaultPath()
339+
path, err := cli.VaultPath()
340340
if err != nil {
341-
t.Fatalf("vaultPath() error = %v", err)
341+
t.Fatalf("cli.VaultPath() error = %v", err)
342342
}
343343
if path != "/test/vault" {
344-
t.Errorf("vaultPath() = %q, want %q", path, "/test/vault")
344+
t.Errorf("cli.VaultPath() = %q, want %q", path, "/test/vault")
345345
}
346346
}
347347

@@ -371,7 +371,7 @@ func TestUnlockVaultSavesToKeyring(t *testing.T) {
371371
vaultFlag.Changed = false
372372
}
373373

374-
v, err := unlockVault(vaultDir, false)
374+
v, err := cli.UnlockVault(vaultDir, false)
375375
if err != nil {
376376
t.Fatalf("unlockVault failed: %v", err)
377377
}

‎cmd/cmd_test.go‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -37,13 +37,13 @@ func TestExpandVaultDir(t *testing.T) {
3737

3838
for _, tt := range tests {
3939
t.Run(tt.input, func(t *testing.T) {
40-
got, err := expandVaultDir(tt.input)
40+
got, err := cli.ExpandVaultDir(tt.input)
4141
if (err != nil) != tt.wantErr {
42-
t.Errorf("expandVaultDir() error = %v, wantErr %v", err, tt.wantErr)
42+
t.Errorf("cli.ExpandVaultDir() error = %v, wantErr %v", err, tt.wantErr)
4343
return
4444
}
4545
if got != tt.expected {
46-
t.Errorf("expandVaultDir() = %q, want %q", got, tt.expected)
46+
t.Errorf("cli.ExpandVaultDir() = %q, want %q", got, tt.expected)
4747
}
4848
})
4949
}
@@ -147,12 +147,12 @@ func TestVaultPathWithTilde(t *testing.T) {
147147
_ = os.Setenv("HOME", "/custom/home")
148148

149149
cli.Vault = "~/my-vault"
150-
got, _ := vaultPath()
150+
got, _ := cli.VaultPath()
151151

152152
home, _ := os.UserHomeDir()
153153
expected := filepath.Join(home, "my-vault")
154154
if got != expected {
155-
t.Errorf("vaultPath() = %q, want %q", got, expected)
155+
t.Errorf("cli.VaultPath() = %q, want %q", got, expected)
156156
}
157157
}
158158

@@ -161,21 +161,21 @@ func TestVaultPathWithAbsolute(t *testing.T) {
161161
t.Skip("skipping on windows: path format differs")
162162
}
163163
cli.Vault = "/absolute/path"
164-
got, _ := vaultPath()
164+
got, _ := cli.VaultPath()
165165

166166
if got != "/absolute/path" {
167-
t.Errorf("vaultPath() = %q, want %q", got, "/absolute/path")
167+
t.Errorf("cli.VaultPath() = %q, want %q", got, "/absolute/path")
168168
}
169169
}
170170

171171
func TestVaultPathWithTildeOnly(t *testing.T) {
172172
home, _ := os.UserHomeDir()
173173
cli.Vault = "~"
174-
got, _ := vaultPath()
174+
got, _ := cli.VaultPath()
175175

176176
expected := home
177177
if got != expected {
178-
t.Errorf("vaultPath() = %q, want %q", got, expected)
178+
t.Errorf("cli.VaultPath() = %q, want %q", got, expected)
179179
}
180180
}
181181

@@ -207,7 +207,7 @@ func TestUnlockVaultLocked(t *testing.T) {
207207
_ = os.Setenv("OPENPASS_VAULT", vaultDir)
208208
defer func() { _ = os.Unsetenv("OPENPASS_VAULT") }()
209209

210-
_, err := unlockVault(vaultDir, false)
210+
_, err := cli.UnlockVault(vaultDir, false)
211211
if err == nil {
212212
t.Error("expected error for locked vault")
213213
}

‎cmd/device.go‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -54,12 +54,12 @@ you must run 'openpass device accept <token>' to re-encrypt all entries
5454
for the new device.`,
5555
Example: ` openpass device pair`,
5656
RunE: func(cmd *cobra.Command, args []string) error {
57-
vaultDir, err := vaultPath()
57+
vaultDir, err := cli.VaultPath()
5858
if err != nil {
5959
return err
6060
}
6161

62-
v, err := unlockVault(vaultDir, true)
62+
v, err := cli.UnlockVault(vaultDir, true)
6363
if err != nil {
6464
return err
6565
}
@@ -114,7 +114,7 @@ to re-encrypt all entries for this new device.`,
114114
remoteURL := args[0]
115115
token := strings.TrimSpace(args[1])
116116

117-
vaultDir, err := vaultPath()
117+
vaultDir, err := cli.VaultPath()
118118
if err != nil {
119119
return err
120120
}
@@ -144,7 +144,7 @@ to re-encrypt all entries for this new device.`,
144144

145145
fmt.Fprintf(os.Stderr, "Pairing with device (public key: %s)\n", truncatePubkey(pf.PublicKey))
146146

147-
passphrase, err := readHiddenInput("Enter passphrase for this device (minimum 12 characters): ", nil)
147+
passphrase, err := cli.ReadHiddenInput("Enter passphrase for this device (minimum 12 characters): ", nil)
148148
if err != nil {
149149
return fmt.Errorf("read passphrase: %w", err)
150150
}
@@ -243,12 +243,12 @@ can decrypt them.`,
243243
RunE: func(cmd *cobra.Command, args []string) error {
244244
token := strings.TrimSpace(args[0])
245245

246-
vaultDir, err := vaultPath()
246+
vaultDir, err := cli.VaultPath()
247247
if err != nil {
248248
return err
249249
}
250250

251-
v, err := unlockVault(vaultDir, true)
251+
v, err := cli.UnlockVault(vaultDir, true)
252252
if err != nil {
253253
return err
254254
}
@@ -306,7 +306,7 @@ any registered device (unmanaged recipients).`,
306306
Example: ` openpass device list
307307
openpass device list --output json`,
308308
RunE: func(cmd *cobra.Command, args []string) error {
309-
vaultDir, err := vaultPath()
309+
vaultDir, err := cli.VaultPath()
310310
if err != nil {
311311
return err
312312
}
@@ -426,7 +426,7 @@ request so the first device can accept it.`,
426426

427427
raw := strings.TrimSpace(args[0])
428428

429-
vaultDir, err := vaultPath()
429+
vaultDir, err := cli.VaultPath()
430430
if err != nil {
431431
return err
432432
}
@@ -448,7 +448,7 @@ request so the first device can accept it.`,
448448
return fmt.Errorf("invalid public key in pairing data: expected age1... format")
449449
}
450450

451-
passphrase, err := readHiddenInput("Enter passphrase for this device (minimum 12 characters): ", nil)
451+
passphrase, err := cli.ReadHiddenInput("Enter passphrase for this device (minimum 12 characters): ", nil)
452452
if err != nil {
453453
return fmt.Errorf("read passphrase: %w", err)
454454
}
@@ -547,7 +547,7 @@ access to all vault entries.`,
547547
RunE: func(cmd *cobra.Command, args []string) error {
548548
deviceName := args[0]
549549

550-
vaultDir, err := vaultPath()
550+
vaultDir, err := cli.VaultPath()
551551
if err != nil {
552552
return err
553553
}
@@ -558,7 +558,7 @@ access to all vault entries.`,
558558
errorspkg.ErrVaultNotInitialized)
559559
}
560560

561-
v, err := unlockVault(vaultDir, true)
561+
v, err := cli.UnlockVault(vaultDir, true)
562562
if err != nil {
563563
return err
564564
}

‎cmd/dynamic.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77

88
"github.com/spf13/cobra"
99

10+
cli "github.com/danieljustus/OpenPass/internal/cli"
1011
"github.com/danieljustus/OpenPass/internal/dynamicsecret"
1112
vaultsvc "github.com/danieljustus/OpenPass/internal/vaultsvc"
1213
)
@@ -41,7 +42,7 @@ var dynamicGenerateCmd = &cobra.Command{
4142
# Generate AWS STS credentials for a specific role
4243
openpass dynamic generate --engine aws-sts --role arn:aws:iam::123456789012:role/MyRole --ttl 30m`,
4344
RunE: func(cmd *cobra.Command, args []string) error {
44-
return withVault(func(svc vaultsvc.Service) error {
45+
return cli.WithVault(func(svc vaultsvc.Service) error {
4546
ctx := context.Background()
4647
mgr := dynamicsecret.NewManager(svc)
4748

‎cmd/generate.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ var generateCmd = &cobra.Command{
4141
}
4242

4343
if genStore != "" {
44-
return withVaultRaw(func(v *vaultpkg.Vault) error {
44+
return cli.WithVaultRaw(func(v *vaultpkg.Vault) error {
4545
entryPath := vaultpkg.EntryPath(v, genStore)
4646
if _, err := vaultpkg.ReadEntry(v.Dir, genStore, v.Identity); err == nil {
4747
if _, err := vaultpkg.MergeEntryWithRecipients(v.Dir, genStore, map[string]any{"password": password}, v.Identity); err != nil {

‎cmd/git.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55

66
"github.com/spf13/cobra"
77

8+
cli "github.com/danieljustus/OpenPass/internal/cli"
89
errorspkg "github.com/danieljustus/OpenPass/internal/errors"
910
"github.com/danieljustus/OpenPass/internal/git"
1011
vaultpkg "github.com/danieljustus/OpenPass/internal/vault"
@@ -24,7 +25,7 @@ var gitCmd = &cobra.Command{
2425
action := args[0]
2526

2627
if action == "log" {
27-
return withVaultRaw(func(v *vaultpkg.Vault) error {
28+
return cli.WithVaultRaw(func(v *vaultpkg.Vault) error {
2829
path := ""
2930
if len(args) > 1 {
3031
path = args[1]
@@ -41,7 +42,7 @@ var gitCmd = &cobra.Command{
4142
})
4243
}
4344

44-
vaultDir, err := vaultPath()
45+
vaultDir, err := cli.VaultPath()
4546
if err != nil {
4647
return err
4748
}

‎cmd/policy.go‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77

88
"github.com/spf13/cobra"
99

10+
cli "github.com/danieljustus/OpenPass/internal/cli"
1011
"github.com/danieljustus/OpenPass/internal/policy"
1112
)
1213

@@ -105,7 +106,7 @@ Example:
105106
return err
106107
}
107108

108-
vaultDir, _ := vaultPath()
109+
vaultDir, _ := cli.VaultPath()
109110
policiesDir := filepath.Join(vaultDir, "policies")
110111
_ = os.MkdirAll(policiesDir, 0750)
111112

@@ -133,7 +134,7 @@ var policyListCmd = &cobra.Command{
133134
requiresVaultAnnotation: "false",
134135
},
135136
RunE: func(cmd *cobra.Command, args []string) error {
136-
vaultDir, _ := vaultPath()
137+
vaultDir, _ := cli.VaultPath()
137138
policiesDir := filepath.Join(vaultDir, "policies")
138139

139140
entries, err := os.ReadDir(policiesDir)
@@ -169,7 +170,7 @@ var policyRemoveCmd = &cobra.Command{
169170
requiresVaultAnnotation: "false",
170171
},
171172
RunE: func(cmd *cobra.Command, args []string) error {
172-
vaultDir, _ := vaultPath()
173+
vaultDir, _ := cli.VaultPath()
173174
policiesDir := filepath.Join(vaultDir, "policies")
174175
policyPath := filepath.Join(policiesDir, args[0])
175176

‎cmd/recipients.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ var recipientsListCmd = &cobra.Command{
3535
Long: `List all recipients from the recipients.txt file.`,
3636
Example: ` openpass recipients list`,
3737
RunE: func(cmd *cobra.Command, args []string) error {
38-
vaultDir, err := vaultPath()
38+
vaultDir, err := cli.VaultPath()
3939
if err != nil {
4040
return err
4141
}
@@ -98,7 +98,7 @@ Once added, all new entries will be encrypted for this recipient.`,
9898
Example: ` openpass recipients add age1ql3z7hjy54pw3hyww5ayyfg7zqgvc7w3j2elw8zmrj2kg5sfn9aqmcac8p`,
9999
Args: cobra.ExactArgs(1),
100100
RunE: func(cmd *cobra.Command, args []string) error {
101-
return withVaultRaw(func(v *vaultpkg.Vault) error {
101+
return cli.WithVaultRaw(func(v *vaultpkg.Vault) error {
102102
recipient := args[0]
103103

104104
rm := vaultpkg.NewRecipientsManager(v.Dir)
@@ -130,7 +130,7 @@ Use --yes to skip confirmation (useful for scripts).`,
130130
Example: ` openpass recipients remove age1ql3z7hjy54pw3hyww5ayyfg7zqgvc7w3j2elw8zmrj2kg5sfn9aqmcac8p`,
131131
Args: cobra.ExactArgs(1),
132132
RunE: func(cmd *cobra.Command, args []string) error {
133-
return withVaultRaw(func(v *vaultpkg.Vault) error {
133+
return cli.WithVaultRaw(func(v *vaultpkg.Vault) error {
134134
recipient := args[0]
135135

136136
confirmed, err := confirmInteractive(fmt.Sprintf("Remove recipient %s", recipient), confirmRemove)

‎cmd/remote.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ func runRemoteInit(cmd *cobra.Command, args []string) error {
9292
return errorspkg.NewCLIError(errorspkg.ExitGeneralError, "ssh-target must not be empty", nil)
9393
}
9494

95-
vaultDir, err := vaultPath()
95+
vaultDir, err := cli.VaultPath()
9696
if err != nil {
9797
return err
9898
}
@@ -157,7 +157,7 @@ func runRemoteInit(cmd *cobra.Command, args []string) error {
157157
}
158158

159159
func runRemoteStatus(cmd *cobra.Command, args []string) error {
160-
vaultDir, err := vaultPath()
160+
vaultDir, err := cli.VaultPath()
161161
if err != nil {
162162
return err
163163
}

‎cmd/root.go‎

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -30,18 +30,6 @@ var (
3030

3131
var rootCmd = cli.RootCmd
3232

33-
// Aliases for functions that moved to internal/cli/ but are still used by staying cmd/ files
34-
var (
35-
vaultPath = cli.VaultPath
36-
unlockVault = cli.UnlockVault
37-
readHiddenInput = cli.ReadHiddenInput
38-
expandVaultDir = cli.ExpandVaultDir
39-
defaultSessionTTL = cli.DefaultSessionTTL
40-
withVault = cli.WithVault
41-
withVaultRaw = cli.WithVaultRaw
42-
Version = cli.AppVersion
43-
)
44-
4533
func Execute() {
4634
cli.Execute()
4735
}

0 commit comments

Comments
 (0)