Skip to content
Merged
89 changes: 56 additions & 33 deletions src/index.test.ts
Original file line number Diff line number Diff line change
@@ -1,18 +1,26 @@
import { setupServer, SetupServerApi } from "msw/node";
import { http, HttpResponse } from "msw";
import { CustomDnsResolver, getDocumentStoreRecords, queryDns, parseDocumentStoreResults, getDnsDidRecords } from ".";
import {
aliDnsResolver,
cloudflareDnsResolver,
getDocumentStoreRecords,
getDnsDidRecords,
googleDnsResolver,
parseDocumentStoreResults,
queryDns,
} from ".";
import { DnsproveStatusCode } from "./common/error";

describe("getCertStoreRecords", () => {
const sampleDnsTextRecordWithDnssec = {
type: "openatts",
net: "ethereum",
netId: "3",
dnssec: true,
dnssec: false,
addr: "0x2f60375e8144e16Adf1979936301D8341D58C36C",
};
test("it should work", async () => {
const records = await getDocumentStoreRecords("donotuse.openattestation.com");
const records = await getDocumentStoreRecords("donotuse.trustvc.io");
expect(records).toStrictEqual([sampleDnsTextRecordWithDnssec]);
Comment on lines +23 to 24
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

These tests are still coupled to mutable public DNS data.

Switching to donotuse.trustvc.io and updating the Astron fixture fixes the current failures, but these cases still hit live zones and assert the full TXT record set. This PR already shows the failure mode: upstream DNS changed, so the test had to be edited. Please move these expectations behind fixtures/MSW, or at least assert only the invariant record(s) with arrayContaining and keep live-DNS checks as separate integration coverage.

Also applies to: 38-47, 340-365

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/index.test.ts` around lines 23 - 24, The test in src/index.test.ts
asserts full live DNS TXT records returned by
getDocumentStoreRecords("donotuse.trustvc.io"), which couples tests to mutable
public DNS; change the spec to use a controlled fixture or MSW to mock DNS
responses for deterministic unit tests, or at minimum replace strict equality
assertions (expect(...).toStrictEqual([...sampleDnsTextRecordWithDnssec])) with
partial assertions using arrayContaining/expect.objectContaining to only assert
invariant fields for the TXT record(s); keep any full-live-DNS assertions in a
separate integration test file or guarded by an environment flag.

});

Expand All @@ -27,14 +35,14 @@ describe("getCertStoreRecords", () => {

describe("getDnsDidRecords", () => {
test("it should work", async () => {
const records = await getDnsDidRecords("donotuse.openattestation.com");
const records = await getDnsDidRecords("donotuse.trustvc.io");
expect(records).toStrictEqual([
{
type: "openatts",
algorithm: "dns-did",
publicKey: "did:ethr:0xE712878f6E8d5d4F9e87E10DA604F9cB564C9a89#controller",
version: "1.0",
dnssec: true,
dnssec: false,
},
]);
});
Expand Down Expand Up @@ -177,29 +185,29 @@ describe("queryDns", () => {
RA: true,
AD: true,
CD: false,
Question: [{ name: "donotuse.openattestation.com.", type: 16 }],
Question: [{ name: "donotuse.trustvc.io.", type: 16 }],
Answer: [
{
name: "donotuse.openattestation.com.",
name: "donotuse.trustvc.io.",
type: 16,
TTL: 300,
data: "openatts a=dns-did; p=did:ethr:0xE712878f6E8d5d4F9e87E10DA604F9cB564C9a89#controller; v=1.0;",
},
{
name: "donotuse.openattestation.com.",
name: "donotuse.trustvc.io.",
type: 16,
TTL: 300,
data:
"openatts DO NOT ADD ANY RECORDS BEYOND THIS AS THIS DOMAIN IS USED FOR DNSPROVE NPM LIBRARY INTEGRATION TESTS",
},
{
name: "donotuse.openattestation.com.",
name: "donotuse.trustvc.io.",
type: 16,
TTL: 300,
data: "openatts fooooooobarrrrrrrrr this entry exists to ensure validation works",
},
{
name: "donotuse.openattestation.com.",
name: "donotuse.trustvc.io.",
type: 16,
TTL: 300,
data: "openatts net=ethereum netId=3 addr=0x2f60375e8144e16Adf1979936301D8341D58C36C",
Expand All @@ -208,22 +216,7 @@ describe("queryDns", () => {
Comment: "Response from 205.251.199.177.",
};

const testDnsResolvers: CustomDnsResolver[] = [
async (domain) => {
const data = await fetch(`https://dns.google/resolve?name=${domain}&type=TXT`, {
method: "GET",
});

return data.json();
},
async (domain) => {
const data = await fetch(`https://cloudflare-dns.com/dns-query?name=${domain}&type=TXT`, {
method: "GET",
headers: { accept: "application/dns-json", contentType: "application/json", connection: "keep-alive" },
});
return data.json();
},
];
const testDnsResolvers = [googleDnsResolver, cloudflareDnsResolver, aliDnsResolver];

afterEach(() => {
server.close();
Expand All @@ -239,7 +232,7 @@ describe("queryDns", () => {
server = setupServer(...handlers);
server.listen();

const records = await queryDns("https://donotuse.openattestation.com", testDnsResolvers);
const records = await queryDns("https://donotuse.trustvc.io", testDnsResolvers);
const sortedAnswer = records?.Answer.sort((a, b) => a.data.localeCompare(b.data));
expect(sortedAnswer).toMatchObject(sampleResponse.Answer);
});
Expand All @@ -256,7 +249,28 @@ describe("queryDns", () => {
server = setupServer(...handlers);
server.listen();

const records = await queryDns("https://donotuse.openattestation.com", testDnsResolvers);
const records = await queryDns("https://donotuse.trustvc.io", testDnsResolvers);

const sortedAnswer = records?.Answer.sort((a, b) => a.data.localeCompare(b.data));
expect(sortedAnswer).toMatchObject(sampleResponse.Answer);
});

test("Should fallback to third dns when first and second dns is down", async () => {
const handlers = [
http.get("https://dns.google/resolve", (_) => {
return new HttpResponse(null, { status: 500 });
}),
http.get("https://cloudflare-dns.com/dns-query", (_) => {
return new HttpResponse(null, { status: 500 });
}),
http.get("https://dns.alidns.com/resolve", (_) => {
return HttpResponse.json(sampleResponse);
}),
];
server = setupServer(...handlers);
server.listen();

const records = await queryDns("https://donotuse.trustvc.io", testDnsResolvers);

const sortedAnswer = records?.Answer.sort((a, b) => a.data.localeCompare(b.data));
expect(sortedAnswer).toMatchObject(sampleResponse.Answer);
Expand All @@ -270,14 +284,16 @@ describe("queryDns", () => {
http.get("https://cloudflare-dns.com/dns-query", (_) => {
return new HttpResponse(null, { status: 500 });
}),
http.get("https://dns.alidns.com/resolve", (_) => {
return new HttpResponse(null, { status: 500 });
}),
];
server = setupServer(...handlers);
server.listen();
try {
await queryDns("https://donotuse.openattestation.com", testDnsResolvers);
} catch (e: any) {
expect(e.code).toStrictEqual(DnsproveStatusCode.IDNS_QUERY_ERROR_GENERAL);
}

await expect(queryDns("https://donotuse.trustvc.io", testDnsResolvers)).rejects.toMatchObject({
code: DnsproveStatusCode.IDNS_QUERY_ERROR_GENERAL,
});
});
});

Expand Down Expand Up @@ -321,6 +337,13 @@ describe("getDocumentStoreRecords for Astron", () => {
addr: "0x18bc0127Ae33389cD96593a1a612774fD14c0737",
dnssec: false,
},
{
type: "openatts",
net: "ethereum",
netId: "1338",
addr: "0x94FD21A026E29E0686583b8be71Cb28a8ca1A8d4",
dnssec: false,
},
{
type: "openatts",
net: "ethereum",
Expand Down
31 changes: 11 additions & 20 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { OpenAttestationDNSTextRecord, OpenAttestationDNSTextRecordT } from "./r
import { OpenAttestationDnsDidRecord, OpenAttestationDnsDidRecordT } from "./records/dnsDid";
import { getLogger } from "./util/logger";
import { CodedError, DnsproveStatusCode } from "./common/error";
import { aliDnsResolver, cloudflareDnsResolver, googleDnsResolver } from "./util/dns-resolvers";

const { trace } = getLogger("index");

Expand All @@ -23,22 +24,7 @@ interface GenericObject {

export type CustomDnsResolver = (domain: string) => Promise<IDNSQueryResponse>;

export const defaultDnsResolvers: CustomDnsResolver[] = [
async (domain) => {
const data = await fetch(`https://dns.google/resolve?name=${domain}&type=TXT`, {
method: "GET",
});

return data.json();
},
async (domain) => {
const data = await fetch(`https://cloudflare-dns.com/dns-query?name=${domain}&type=TXT`, {
method: "GET",
headers: { accept: "application/dns-json", contentType: "application/json", connection: "keep-alive" },
});
return data.json();
},
];
export const defaultDnsResolvers: CustomDnsResolver[] = [googleDnsResolver, cloudflareDnsResolver, aliDnsResolver];

/**
* Returns true for strings that are openattestation records
Expand Down Expand Up @@ -115,8 +101,10 @@ export const parseOpenAttestationRecord = (record: string): GenericObject => {
trace(`Parsing record: ${record}`);
const keyValuePairs = record.trim().split(" "); // tokenize into key=value elements
const recordObject = {} as GenericObject;
// @ts-ignore: we already checked for this token
recordObject.type = keyValuePairs.shift();
const typeToken = keyValuePairs.shift();
if (typeToken !== undefined) {
recordObject.type = typeToken;
}
keyValuePairs.reduce<GenericObject>(addKeyValuePairToObject, recordObject);
return recordObject;
};
Expand Down Expand Up @@ -153,6 +141,7 @@ const parseOpenAttestationRecords = (recordSet: IDNSRecord[] = []): GenericObjec
/**
* Takes a DNS-TXT Record set and returns openattestation document store records if any
* @param recordSet Refer to tests for examples
* @param dnssec Resolver AD (authenticated data) flag; applied as each record's `dnssec` field
*/
export const parseDocumentStoreResults = (
recordSet: IDNSRecord[] = [],
Expand All @@ -177,6 +166,7 @@ export const parseDnsDidResults = (recordSet: IDNSRecord[] = [], dnssec: boolean
/**
* Queries a given domain and parses the results to retrieve openattestation document store records if any
* @param domain e.g: "example.openattestation.com"
* @param customDnsResolvers Optional resolver list; built-in HTTP DNS chain is used when omitted
* @example
* > getDocumentStoreRecords("example.openattestation.com")
* > [ { type: 'openatts',
Expand All @@ -191,7 +181,7 @@ export const getDocumentStoreRecords = async (
): Promise<OpenAttestationDNSTextRecord[]> => {
trace(`Received request to resolve ${domain}`);

const dnsResolvers = customDnsResolvers || defaultDnsResolvers;
const dnsResolvers = customDnsResolvers ?? defaultDnsResolvers;

const results = await queryDns(domain, dnsResolvers);
const answers = results.Answer || [];
Expand All @@ -207,7 +197,7 @@ export const getDnsDidRecords = async (
): Promise<OpenAttestationDnsDidRecord[]> => {
trace(`Received request to resolve ${domain}`);

const dnsResolvers = customDnsResolvers || defaultDnsResolvers;
const dnsResolvers = customDnsResolvers ?? defaultDnsResolvers;

const results = await queryDns(domain, dnsResolvers);
const answers = results.Answer || [];
Expand All @@ -218,3 +208,4 @@ export const getDnsDidRecords = async (
};

export { OpenAttestationDNSTextRecord, OpenAttestationDnsDidRecord };
export * from "./util/dns-resolvers";
29 changes: 29 additions & 0 deletions src/util/dns-resolvers/ali-dns-resolver.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import type { CustomDnsResolver, IDNSQueryResponse } from "../..";

/** Ali DNS JSON API uses numeric RRTYPE; 16 = TXT */
const ALI_DNS_TXT_QUERY_TYPE = "16";
export const aliDnsResolver: CustomDnsResolver = async (domain) => {
const url = new URL("https://dns.alidns.com/resolve");

if (!domain) {
throw new Error("Domain is required");
}

url.searchParams.set("name", domain);
url.searchParams.set("type", ALI_DNS_TXT_QUERY_TYPE);

const res = await fetch(url);

if (!res.ok) {
throw new Error(`Ali DNS request failed: HTTP ${res.status}`);
}

let data;
try {
data = await res.json();
} catch {
throw new Error("Failed to parse DNS response JSON");
}

return data as IDNSQueryResponse;
};
29 changes: 29 additions & 0 deletions src/util/dns-resolvers/cloudflare-dns-resolver.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import type { CustomDnsResolver, IDNSQueryResponse } from "../..";

export const cloudflareDnsResolver: CustomDnsResolver = async (domain) => {
const url = new URL("https://cloudflare-dns.com/dns-query");

if (!domain) {
throw new Error("Domain is required");
}

url.searchParams.set("name", domain);
url.searchParams.set("type", "TXT");
Comment on lines +6 to +11
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Harden domain validation for whitespace-only input.

At Line 6, if (!domain) allows values like " ", which then get sent as a DNS query name at Line 10. Trim before validation and use the normalized value in searchParams.

🔧 Suggested patch
 export const cloudflareDnsResolver: CustomDnsResolver = async (domain) => {
   const url = new URL("https://cloudflare-dns.com/dns-query");
+  const normalizedDomain = domain?.trim();

-  if (!domain) {
+  if (!normalizedDomain) {
     throw new Error("Domain is required");
   }

-  url.searchParams.set("name", domain);
+  url.searchParams.set("name", normalizedDomain);
   url.searchParams.set("type", "TXT");
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (!domain) {
throw new Error("Domain is required");
}
url.searchParams.set("name", domain);
url.searchParams.set("type", "TXT");
export const cloudflareDnsResolver: CustomDnsResolver = async (domain) => {
const url = new URL("https://cloudflare-dns.com/dns-query");
const normalizedDomain = domain?.trim();
if (!normalizedDomain) {
throw new Error("Domain is required");
}
url.searchParams.set("name", normalizedDomain);
url.searchParams.set("type", "TXT");
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/util/dns-resolvers/cloudflare-dns-resolver.ts` around lines 6 - 11, Trim
and validate the incoming domain before using it: in the Cloudflare DNS resolver
where `domain` is checked and `url.searchParams` are set (the code around the
`if (!domain)` check and the subsequent `url.searchParams.set("name", domain)` /
`url.searchParams.set("type", "TXT")` lines), replace the raw `domain` usage
with a trimmed/normalized value (e.g., `const normalized = domain?.trim()`),
throw an error if `normalized` is falsy/empty, and then use that `normalized`
value when calling `url.searchParams.set("name", normalized)` so whitespace-only
inputs are rejected and not sent to the DNS query.


const res = await fetch(url, {
headers: { Accept: "application/dns-json" },
});

if (!res.ok) {
throw new Error(`Cloudflare DNS request failed: HTTP ${res.status}`);
}

let data;
try {
data = await res.json();
} catch {
throw new Error("Failed to parse DNS response JSON");
}

return data as IDNSQueryResponse;
};
Loading
Loading