Skip to content

Conversation

@hono0130
Copy link
Contributor

@hono0130 hono0130 commented Nov 20, 2025

What this PR does:

Fix nil pointer dereference in event watcher when repository re-clone fails.

Why we need it:

During the GitHub service disruption on 2025-11-18 (https://www.githubstatus.com/incidents/5q7nmlxz30sk), piped crashed with a nil pointer panic:

  panic: runtime error: invalid memory address or nil pointer dereference
  [signal SIGSEGV: segmentation violation code=0x1 addr=0x58 pc=0x20cc9fc]

  goroutine 1769 [running]:
    github.com/pipe-cd/pipecd/pkg/app/piped/eventwatcher.(*watcher).run(
      0xc0003f2000,
      {0x2ca7548, 0xc00070a640},
      {0x2cb7570, 0xc005c93b00},
      {{0xc000058738, 0x13}, {0xc0000574d0, 0x2c}, {0xc0004f726a, ...}},
    )
      /home/runner/work/pipecd/pipecd/pkg/app/piped/eventwatcher/eventwatcher.go:185 +0x41c
  created by github.com/pipe-cd/pipecd/pkg/app/piped/eventwatcher.(*watcher).Run in goroutine 112
      /home/runner/work/pipecd/pipecd/pkg/app/piped/eventwatcher/eventwatcher.go:145 +0x2ea

When repo.Pull() fails, the event watcher tries to re-clone the repository. However, the original code had a bug:

repo, err = w.cloneRepo(ctx, repoCfg)  // repo becomes nil if this fails
if err != nil {
    w.logger.Error("failed to re-clone repository", ...)
    // continues with repo == nil, causing panic on next iteration
}

This fix ensures the repo variable is only updated when re-cloning succeeds:

newRepo, err := w.cloneRepo(ctx, repoCfg)
if err != nil {
    w.logger.Error("failed to re-clone repository", ...)
} else {
    repo = newRepo  // Only update on success
}

Does this PR introduce a user-facing change?:

  • How are users affected by this change:
    Piped no longer crashes during git service disruptions. The event watcher continues retrying instead of panicking.

  • Is this breaking change:
    No

Signed-off-by: hono0130 <toda_honoka@cyberagent.co.jp>
Copy link
Member

@khanhtc1202 khanhtc1202 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch, thank you 💯

@khanhtc1202 khanhtc1202 merged commit 3763d44 into pipe-cd:master Nov 20, 2025
43 checks passed
@github-actions github-actions bot mentioned this pull request Dec 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants