Skip to content

fix: tighten recovery target validation #10565

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mnencia merged 1 commit into from
May 7, 2026
Merged

fix: tighten recovery target validation #10565

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions internal/webhook/v1/cluster_webhook.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1406,6 +1406,39 @@ func (v *ClusterCustomValidator) validateRecoveryTarget(r *apiv1.Cluster) field.
}
}

// PostgreSQL casts recovery_target_xid to TransactionId (uint32); a value
// above 2^32-1 would be silently truncated. Use bitSize 32 so we reject
// rather than admit a different XID than the user wrote.
if recoveryTarget.TargetXID != "" {
if _, err := strconv.ParseUint(recoveryTarget.TargetXID, 10, 32); err != nil {
result = append(result, field.Invalid(
field.NewPath("spec", "bootstrap", "recovery", "recoveryTarget", "targetXID"),
recoveryTarget.TargetXID,
"recovery target xid must be a non-negative 32-bit integer"))
}
}

// PostgreSQL enforces MAXFNAMELEN (64) on recovery_target_name and on
// pg_create_restore_point; mirror it at admission for a better error.
if len(recoveryTarget.TargetName) >= 64 {
result = append(result, field.Invalid(
field.NewPath("spec", "bootstrap", "recovery", "recoveryTarget", "targetName"),
recoveryTarget.TargetName,
"recovery target name must be shorter than 64 bytes"))
}

// pg_create_restore_point accepts arbitrary text, but a name with
// newlines or NUL is never legitimate and is a strong signal of a
// malformed or hostile spec.
if strings.ContainsFunc(recoveryTarget.TargetName, func(r rune) bool {
return r < 0x20 || r == 0x7F
}) {
result = append(result, field.Invalid(
field.NewPath("spec", "bootstrap", "recovery", "recoveryTarget", "targetName"),
recoveryTarget.TargetName,
"recovery target name must not contain ASCII control characters"))
}

// When using a backup catalog, we can identify the backup to be restored
// only if the PITR is time-based. If the PITR is not time-based, the user
// need to specify a backup ID.
Expand Down
84 changes: 83 additions & 1 deletion internal/webhook/v1/cluster_webhook_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1562,7 +1562,7 @@ var _ = Describe("recovery target", func() {
RecoveryTarget: &apiv1.RecoveryTarget{
BackupID: "",
TargetTLI: "",
TargetXID: "1/1",
TargetXID: "1234",
TargetName: "",
TargetLSN: "",
TargetTime: "",
Expand Down Expand Up @@ -1779,6 +1779,88 @@ var _ = Describe("recovery target", func() {
Expect(v.validateRecoveryTarget(cluster)).To(HaveLen(1))
})
})

When("TargetXID is specified", func() {
recoveryTargetWith := func(xid string) *apiv1.Cluster {
return &apiv1.Cluster{
Spec: apiv1.ClusterSpec{
Bootstrap: &apiv1.BootstrapConfiguration{
Recovery: &apiv1.BootstrapRecovery{
RecoveryTarget: &apiv1.RecoveryTarget{
BackupID: "backup-id",
TargetXID: xid,
},
},
},
},
}
}

It("accepts a non-negative integer", func() {
Expect(v.validateRecoveryTarget(recoveryTargetWith("1234"))).To(BeEmpty())
})

It("rejects a non-numeric value", func() {
Expect(v.validateRecoveryTarget(recoveryTargetWith("not-a-number"))).To(HaveLen(1))
})

It("rejects an LSN-shaped value", func() {
Expect(v.validateRecoveryTarget(recoveryTargetWith("1/1"))).To(HaveLen(1))
})

It("rejects a negative value", func() {
Expect(v.validateRecoveryTarget(recoveryTargetWith("-1"))).To(HaveLen(1))
})

It("accepts the largest 32-bit XID", func() {
Expect(v.validateRecoveryTarget(recoveryTargetWith("4294967295"))).To(BeEmpty())
})

It("rejects a value above 2^32-1 to avoid silent epoch truncation", func() {
Expect(v.validateRecoveryTarget(recoveryTargetWith("4294967296"))).To(HaveLen(1))
})
})

When("TargetName is specified", func() {
recoveryTargetWith := func(name string) *apiv1.Cluster {
return &apiv1.Cluster{
Spec: apiv1.ClusterSpec{
Bootstrap: &apiv1.BootstrapConfiguration{
Recovery: &apiv1.BootstrapRecovery{
RecoveryTarget: &apiv1.RecoveryTarget{
BackupID: "backup-id",
TargetName: name,
},
},
},
},
}
}

It("accepts an arbitrary printable string", func() {
Expect(v.validateRecoveryTarget(recoveryTargetWith("my'restore point"))).To(BeEmpty())
})

It("rejects an embedded newline", func() {
Expect(v.validateRecoveryTarget(recoveryTargetWith("line1\nline2"))).To(HaveLen(1))
})

It("rejects an embedded NUL byte", func() {
Expect(v.validateRecoveryTarget(recoveryTargetWith("a\x00b"))).To(HaveLen(1))
})

It("rejects a DEL byte", func() {
Expect(v.validateRecoveryTarget(recoveryTargetWith("a\x7fb"))).To(HaveLen(1))
})

It("accepts a 63-byte name", func() {
Expect(v.validateRecoveryTarget(recoveryTargetWith(strings.Repeat("a", 63)))).To(BeEmpty())
})

It("rejects a 64-byte name", func() {
Expect(v.validateRecoveryTarget(recoveryTargetWith(strings.Repeat("a", 64)))).To(HaveLen(1))
})
})
})

var _ = Describe("primary update strategy", func() {
Expand Down
Loading