fix(dns): match record content in upsertRecord to prevent overwriting existing records

Previously, upsertRecord matched only by type and name, silently
overwriting any existing record of the same type/name with different
content. This caused data loss when users had multiple records of the
same type (e.g. multiple TXT records for SPF + site verification).

Now upsertRecord also checks the record content/target before deciding
to update. If the content differs, a new record is created alongside
the existing one instead of replacing it.

Affected providers: Cloudflare, Porkbun, Infomaniak, OVH.
Tests updated accordingly.
This commit is contained in:
Yash Kumar 2026-09-08 16:59:16 +05:30
parent 1e6ab0fab7
commit 91db4e60c4
8 changed files with 145 additions and 17 deletions

View File

@ -323,9 +323,11 @@ describe("cloudflareClient.upsertRecord", () => {
expect(createInit.method).toBe("POST");
});
it("updates the existing record instead of creating a duplicate", async () => {
it("updates the existing record when content matches", async () => {
mockFetch
.mockResolvedValueOnce(cfSuccess([{ id: "existing-1" }]))
.mockResolvedValueOnce(
cfSuccess([{ id: "existing-1", type: "A", content: "5.6.7.8" }]),
)
.mockResolvedValueOnce(cfSuccess({ id: "existing-1" }));
const result = await cloudflareClient.upsertRecord(config, {
@ -344,6 +346,25 @@ describe("cloudflareClient.upsertRecord", () => {
expect(updateInit.method).toBe("PUT");
});
it("creates a new record when content differs from existing", async () => {
mockFetch
.mockResolvedValueOnce(
cfSuccess([{ id: "existing-1", type: "A", content: "1.1.1.1" }]),
)
.mockResolvedValueOnce(cfSuccess({ id: "new-2" }));
const result = await cloudflareClient.upsertRecord(config, {
zoneId: "zone-1",
type: "A",
name: "app.example.com",
content: "5.6.7.8",
});
expect(result).toEqual({ id: "new-2" });
const [, createInit] = mockFetch.mock.calls[1] as [string, RequestInit];
expect(createInit.method).toBe("POST");
});
it("defaults ttl to 1 (automatic) when not provided", async () => {
mockFetch
.mockResolvedValueOnce(cfSuccess([]))

View File

@ -203,11 +203,11 @@ describe("infomaniakClient.upsertRecord", () => {
});
});
it("updates the existing record instead of creating a duplicate", async () => {
it("updates the existing record when content matches", async () => {
mockFetch
.mockResolvedValueOnce(
ikSuccess([
{ id: 7, type: "A", source: "app", target: "1.1.1.1", ttl: 300 },
{ id: 7, type: "A", source: "app", target: "1.2.3.4", ttl: 300 },
]),
)
.mockResolvedValueOnce(ikSuccess({ id: 7 }));
@ -228,6 +228,28 @@ describe("infomaniakClient.upsertRecord", () => {
expect(lastBody().ttl).toBe(300);
});
it("creates a new record when content differs from existing", async () => {
mockFetch
.mockResolvedValueOnce(
ikSuccess([
{ id: 7, type: "A", source: "app", target: "1.1.1.1", ttl: 300 },
]),
)
.mockResolvedValueOnce(ikSuccess({ id: 50 }));
const result = await infomaniakClient.upsertRecord(config, {
zoneId: "example.com",
type: "A",
name: "app.example.com",
content: "1.2.3.4",
});
expect(result).toEqual({ id: "50" });
const [url, init] = lastCall();
expect(url).toBe("https://api.infomaniak.com/2/zones/example.com/records");
expect(init.method).toBe("POST");
});
it("writes a root dot as the source for an apex record and strips the trailing dot", async () => {
mockFetch
.mockResolvedValueOnce(ikSuccess([]))
@ -249,7 +271,7 @@ describe("infomaniakClient.upsertRecord", () => {
mockFetch
.mockResolvedValueOnce(
ikSuccess([
{ id: 8, type: "A", source, target: "1.1.1.1", ttl: 3600 },
{ id: 8, type: "A", source, target: "1.2.3.4", ttl: 3600 },
]),
)
.mockResolvedValueOnce(ikSuccess({ id: 8 }));

View File

@ -261,9 +261,21 @@ describe("ovhClient.upsertRecord", () => {
expect(calls[2]?.[1].method).toBe("POST");
});
it("updates the existing record instead of creating a duplicate", async () => {
it("updates the existing record when content matches", async () => {
const cfg = freshConfig();
mockApi(ovhSuccess([4]), ovhSuccess(null), ovhSuccess(null));
mockApi(
ovhSuccess([4]),
ovhSuccess({
id: 4,
zone: "example.com",
fieldType: "A",
subDomain: "app",
target: "1.2.3.4",
ttl: 60,
}),
ovhSuccess(null),
ovhSuccess(null),
);
const result = await ovhClient.upsertRecord(cfg, {
zoneId: "example.com",
@ -274,9 +286,38 @@ describe("ovhClient.upsertRecord", () => {
expect(result).toEqual({ id: "4" });
const calls = apiCalls();
expect(calls[1]?.[0]).toContain("/domain/zone/example.com/record/4");
expect(calls[1]?.[1].method).toBe("PUT");
expect(calls[2]?.[0]).toContain("/refresh");
expect(calls[2]?.[0]).toContain("/domain/zone/example.com/record/4");
expect(calls[2]?.[1].method).toBe("PUT");
expect(calls[3]?.[0]).toContain("/refresh");
});
it("creates a new record when content differs from existing", async () => {
const cfg = freshConfig();
mockApi(
ovhSuccess([4]),
ovhSuccess({
id: 4,
zone: "example.com",
fieldType: "A",
subDomain: "app",
target: "1.1.1.1",
ttl: 60,
}),
ovhSuccess({ id: 10 }),
ovhSuccess(null),
);
const result = await ovhClient.upsertRecord(cfg, {
zoneId: "example.com",
type: "A",
name: "app.example.com",
content: "5.6.7.8",
});
expect(result).toEqual({ id: "10" });
const calls = apiCalls();
expect(calls[2]?.[1].method).toBe("POST");
expect(calls[3]?.[0]).toContain("/refresh");
});
it("omits the ttl so OVH applies the zone default", async () => {

View File

@ -121,9 +121,11 @@ describe("porkbunClient.upsertRecord", () => {
expect(lookupUrl).toContain("/dns/retrieveByNameType/example.com/A/");
});
it("edits the existing record instead of creating a duplicate", async () => {
it("edits the existing record when content matches", async () => {
mockFetch
.mockResolvedValueOnce(pbSuccess({ records: [{ id: "existing-1" }] }))
.mockResolvedValueOnce(
pbSuccess({ records: [{ id: "existing-1", content: "5.6.7.8" }] }),
)
.mockResolvedValueOnce(pbSuccess({}));
const result = await porkbunClient.upsertRecord(config, {
@ -138,6 +140,30 @@ describe("porkbunClient.upsertRecord", () => {
expect(editUrl).toContain("/dns/edit/example.com/existing-1");
});
it("creates a new record when content differs from existing", async () => {
mockFetch
.mockResolvedValueOnce(
pbSuccess({ records: [{ id: "existing-1", content: "1.1.1.1" }] }),
)
.mockResolvedValueOnce(pbSuccess({ id: "new-2" }));
const result = await porkbunClient.upsertRecord(config, {
zoneId: "example.com",
type: "A",
name: "app.example.com",
content: "5.6.7.8",
});
expect(result).toEqual({ id: "new-2" });
const [createUrl, createInit] = mockFetch.mock.calls[1] as [
string,
RequestInit,
];
expect(createUrl).toContain("/dns/create/example.com");
const body = JSON.parse(createInit.body as string);
expect(body).toMatchObject({ name: "app", type: "A", content: "5.6.7.8" });
});
it("defaults ttl to 600 when not provided", async () => {
mockFetch
.mockResolvedValueOnce(pbSuccess({ records: [] }))

View File

@ -168,12 +168,16 @@ export const cloudflareClient: DnsClient<CloudflareConfig> = {
ttl: record.ttl ?? 1,
};
const existing = await cfFetch<{ id: string }[]>(
const existing = await cfFetch<
{ id: string; type: string; content: string; priority?: number }[]
>(
config,
`/zones/${record.zoneId}/dns_records?type=${record.type}&name=${encodeURIComponent(record.name)}`,
);
const existingRecord = existing[0];
const existingRecord = existing.find(
(r) => inlinePriority(r) === record.content,
);
if (existingRecord) {
const updated = await cfFetch<{ id: string }>(
config,

View File

@ -175,7 +175,8 @@ export const infomaniakClient: DnsClient<InfomaniakConfig> = {
const match = existing.find(
(candidate) =>
candidate.type === record.type &&
normalizeSource(candidate.source) === source,
normalizeSource(candidate.source) === source &&
unquoteTarget(candidate.target) === record.content,
);
const body = JSON.stringify(recordPayload(record, record.zoneId));

View File

@ -285,7 +285,18 @@ export const ovhClient: DnsClient<OvhConfig> = {
)}&subDomain=${encodeURIComponent(subDomain)}`,
);
const existingId = existing[0];
let existingId: number | undefined;
for (const id of existing) {
const candidate = await ovhFetch<OvhRecord>(
config,
`/domain/zone/${zone}/record/${id}`,
);
if (candidate.target === record.content) {
existingId = id;
break;
}
}
if (existingId !== undefined) {
await ovhFetch(config, `/domain/zone/${zone}/record/${existingId}`, {
method: "PUT",

View File

@ -96,7 +96,9 @@ export const porkbunClient: DnsClient<PorkbunConfig> = {
ttl: record.ttl ?? 600,
};
const existingRecord = existing.records[0];
const existingRecord = existing.records.find(
(r) => r.content === record.content,
);
if (existingRecord) {
await pbFetch(
config,