32c6f5d fix(review): context timeouts, defer cancel, skip malformed tree lines · gordon

M internal/adapters/in/cli/ui/components/selector.go +2 -1


@@ -82,7 +82,8 @@ func (m SelectorModel) View() string {
82	82		if i == m.cursor {
83	83		b.WriteString(styles.Theme.Highlight.Render(cursor + line))
84	84		} else {
85		-	fmt.Fprintf(&b, "%s%s", cursor, line)
	85	+	b.WriteString(cursor)
	86	+	b.WriteString(line)
86	87		}
87	88		b.WriteString("\n")
88	89		}

 		if i == m.cursor {
 			b.WriteString(styles.Theme.Highlight.Render(cursor + line))
 		} else {
-			fmt.Fprintf(&b, "%s%s", cursor, line)
+			b.WriteString(cursor)
+			b.WriteString(line)
 		}
 		b.WriteString("\n")
 	}

M internal/adapters/out/domainsecrets/pass_store.go +11 -4


@@ -33,7 +33,10 @@ type PassStore struct {
33	33
34	34		// NewPassStore creates a new pass-based domain secret store.
35	35		func NewPassStore(log zerowrap.Logger) (*PassStore, error) {
36		-	if err := exec.Command("pass", "version").Run(); err != nil {
	36	+	// Use a timeout so a stalled GPG agent or keyring does not hang startup.
	37	+	probeCtx, probeCancel := context.WithTimeout(context.Background(), 10*time.Second)
	38	+	defer probeCancel()
	39	+	if err := exec.CommandContext(probeCtx, "pass", "version").Run(); err != nil { //nolint:gosec // binary is a constant ("pass"), no user input
37	40		return nil, fmt.Errorf("pass is not available: %w", err)
38	41		}
39	42

@@ -809,8 +812,12 @@ func (s *PassStore) cleanupInsertedPaths(paths []string) {
809	812		}
810	813
811	814		for _, path := range paths {
812		-	ctx, cancel := context.WithTimeout(context.Background(), s.timeout)
813		-	_ = s.passRemove(ctx, path)
814		-	cancel()
	815	+	// Wrap in a closure so defer cancel() runs at the end of each iteration,
	816	+	// not at the end of cleanupInsertedPaths (which would delay all cancels).
	817	+	func(p string) {
	818	+	ctx, cancel := context.WithTimeout(context.Background(), s.timeout)
	819	+	defer cancel()
	820	+	_ = s.passRemove(ctx, p)
	821	+	}(path)
815	822		}
816	823		}

 // NewPassStore creates a new pass-based domain secret store.
 func NewPassStore(log zerowrap.Logger) (*PassStore, error) {
-	if err := exec.Command("pass", "version").Run(); err != nil {
+	// Use a timeout so a stalled GPG agent or keyring does not hang startup.
+	probeCtx, probeCancel := context.WithTimeout(context.Background(), 10*time.Second)
+	defer probeCancel()
+	if err := exec.CommandContext(probeCtx, "pass", "version").Run(); err != nil { //nolint:gosec // binary is a constant ("pass"), no user input
 		return nil, fmt.Errorf("pass is not available: %w", err)
 	}
 	}
 	for _, path := range paths {
-		ctx, cancel := context.WithTimeout(context.Background(), s.timeout)
+		// Wrap in a closure so defer cancel() runs at the end of each iteration,
-		_ = s.passRemove(ctx, path)
+		// not at the end of cleanupInsertedPaths (which would delay all cancels).
-		cancel()
+		func(p string) {
+			ctx, cancel := context.WithTimeout(context.Background(), s.timeout)
+			defer cancel()
+			_ = s.passRemove(ctx, p)
+		}(path)
 	}
 }

M internal/adapters/out/secrets/pass.go +5 -3


@@ -105,8 +105,10 @@ func (p *PassProvider) GetSecret(ctx context.Context, path string) (string, erro
105	105		}
106	106
107	107		// IsAvailable checks if pass is available in the system.
	108	+	// Uses a short timeout so a stalled GPG agent does not hang the caller.
108	109		func (p *PassProvider) IsAvailable() bool {
109		-	cmd := exec.Command("pass", "version")
110		-	err := cmd.Run()
111		-	return err == nil
	110	+	ctx, cancel := context.WithTimeout(context.Background(), p.timeout)
	111	+	defer cancel()
	112	+	cmd := exec.CommandContext(ctx, "pass", "version") //nolint:gosec // binary is a constant ("pass"), no user input
	113	+	return cmd.Run() == nil
112	114		}

 }
 // IsAvailable checks if pass is available in the system.
+// Uses a short timeout so a stalled GPG agent does not hang the caller.
 func (p *PassProvider) IsAvailable() bool {
-	cmd := exec.Command("pass", "version")
+	ctx, cancel := context.WithTimeout(context.Background(), p.timeout)
-	err := cmd.Run()
+	defer cancel()
-	return err == nil
+	cmd := exec.CommandContext(ctx, "pass", "version") //nolint:gosec // binary is a constant ("pass"), no user input
+	return cmd.Run() == nil
 }

M internal/adapters/out/tokenstore/pass.go +10 -4


@@ -236,7 +236,8 @@ func parsePassLsEntries(output string) []treeEntry {
236	236		// The branch character (├ or └, 3 bytes each) immediately precedes them.
237	237		branchByteIdx := idx - 3
238	238		if branchByteIdx < 0 {
239		-	branchByteIdx = 0
	239	+	// Marker is too close to the start — malformed line, skip.
	240	+	continue
240	241		}
241	242		// Count runes in the indentation prefix before the branch char.
242	243		branchRune = utf8.RuneCountInString(line[:branchByteIdx])

@@ -246,7 +247,8 @@ func parsePassLsEntries(output string) []treeEntry {
246	247		// ASCII variant: branch char (\| or `, 1 byte) is immediately before the dashes.
247	248		branchByteIdx := idx - 1
248	249		if branchByteIdx < 0 {
249		-	branchByteIdx = 0
	250	+	// Marker is too close to the start — malformed line, skip.
	251	+	continue
250	252		}
251	253		branchRune = utf8.RuneCountInString(line[:branchByteIdx])
252	254		// Name starts after: branchChar(1B) + -(1B) + -(1B) + SP(1B) = 4B from branch.

@@ -351,8 +353,12 @@ func (s *PassStore) ListTokens(ctx context.Context) ([]domain.Token, error) {
351	353		continue
352	354		}
353	355
354		-	// Try to get the token metadata
355		-	_, token, err := s.GetToken(ctx, subject)
	356	+	// Try to get the token metadata.
	357	+	// Use a fresh per-call context so each GetToken gets the full timeout,
	358	+	// not the shared (potentially exhausted) deadline from the outer ctx.
	359	+	callCtx, callCancel := context.WithTimeout(context.Background(), s.timeout)
	360	+	_, token, err := s.GetToken(callCtx, subject)
	361	+	callCancel()
356	362		if err != nil {
357	363		s.log.Warn().Err(err).Str("subject", subject).Msg("failed to get token")
358	364		continue

 			// The branch character (├ or └, 3 bytes each) immediately precedes them.
 			branchByteIdx := idx - 3
 			if branchByteIdx < 0 {
-				branchByteIdx = 0
+				// Marker is too close to the start — malformed line, skip.
+				continue
 			}
 			// Count runes in the indentation prefix before the branch char.
 			branchRune = utf8.RuneCountInString(line[:branchByteIdx])
 			// ASCII variant: branch char (| or `, 1 byte) is immediately before the dashes.
 			branchByteIdx := idx - 1
 			if branchByteIdx < 0 {
-				branchByteIdx = 0
+				// Marker is too close to the start — malformed line, skip.
+				continue
 			}
 			branchRune = utf8.RuneCountInString(line[:branchByteIdx])
 			// Name starts after: branchChar(1B) + -(1B) + -(1B) + SP(1B) = 4B from branch.
 			continue
 		}
-		// Try to get the token metadata
+		// Try to get the token metadata.
-		_, token, err := s.GetToken(ctx, subject)
+		// Use a fresh per-call context so each GetToken gets the full timeout,
+		// not the shared (potentially exhausted) deadline from the outer ctx.
+		callCtx, callCancel := context.WithTimeout(context.Background(), s.timeout)
+		_, token, err := s.GetToken(callCtx, subject)
+		callCancel()
 		if err != nil {
 			s.log.Warn().Err(err).Str("subject", subject).Msg("failed to get token")
 			continue