Skip to content

Domain fixes #17

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
merged 7 commits into from
Nov 18, 2025
Merged

Domain fixes #17

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
2d2ce81
added types
marcrupt Nov 17, 2025
4ec61f0
domain fixes and qol improvements
marcrupt Nov 17, 2025
4a0ae1a
domain fixes and qol improvements
marcrupt Nov 17, 2025
45a73f9
domain fixes and qol improvements + pr feedback
marcrupt Nov 17, 2025
a9068ef
domain fixes and qol improvements + pr feedback
marcrupt Nov 17, 2025
1396f16
remove readmes
marcrupt Nov 17, 2025
08c29a4
pr feedback
marcrupt Nov 17, 2025
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
9 changes: 9 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,15 @@ This monorepo contains:
- [x] Messages auto-create or use a thread
- [x] Retrieve threads

### Domains (`/domains`)

- [x] Create domain
- [x] List domains
- [x] Retrieve domain
- [x] Delete domain
- [x] Verify domain
- [x] Get domain DNS records

### Tests

- [x] Full API coverage
Expand Down
15 changes: 0 additions & 15 deletions api/README.md
View file
Open in desktop

This file was deleted.

16 changes: 15 additions & 1 deletion api/controllers/DNSController.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { resolveMx } from 'node:dns/promises';
import { resolveMx, resolveTxt } from 'node:dns/promises';
Copy link

@coderabbitai coderabbitai bot Nov 17, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

TXT lookup only inspects the first TXT RR; consider flattening all records

resolveTxt returns string[][], but getDNSTXTRecords currently returns only records[0] ?? []. If the DMARC TXT is not in the first RR, verifyDomainDNS will never see it. Prefer flattening all strings:

 export async function getDNSTXTRecords({
   domain,
 }: {
   domain: string;
 }): Promise<string[]> {
   try {
-    const records = await resolveTxt(domain);
-    return records[0] ?? [];
+    const records = await resolveTxt(domain);
+    return records.flat();
   } catch (error) {
     console.error('Error resolving TXT records:', error);
     return [];
   }
 }

Also applies to: 17-29

🤖 Prompt for AI Agents 
In api/controllers/DNSController.ts around lines 1 and 17-29, getDNSTXTRecords
currently returns only the first TXT RR (records[0] ?? []), but resolveTxt
returns string[][]; update the function to flatten all TXT record arrays (e.g.,
concat/flat the string[][] into a single string[]), then search that flattened
list for the DMARC TXT entry so lookups work even when DMARC is not in the first
RR.

import type { MxRecord } from 'node:dns';

export async function getDNSMXRecords({
Expand All @@ -13,3 +13,17 @@ export async function getDNSMXRecords({
return [];
}
}

export async function getDNSTXTRecords({
domain,
}: {
domain: string;
}): Promise<string[]> {
try {
const records = await resolveTxt(domain);
return records[0] ?? [];
} catch (error) {
console.error('Error resolving TXT records:', error);
return [];
}
}
34 changes: 32 additions & 2 deletions api/controllers/DomainController.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import mongoose, { type HydratedDocument } from "mongoose";
import Domain from "../db/mongo/schemas/Domain";
import { getDNSMXRecords } from "./DNSController";
import { getDNSMXRecords, getDNSTXTRecords } from "./DNSController";
import type IDomain from "../models/Domain";
import { getDomainVerificationStatus } from "./SESController";

export async function createDomain({
organizationId,
Expand Down Expand Up @@ -91,7 +92,36 @@ export async function verifyDomainDNS({
if (mxRecordFound) {
mxRecordFound.status = "verified";
}
domain.verified = mxRecordFound ? true : false;

const dmarcRecord = await getDNSTXTRecords({ domain: `_dmarc.${domain.name}` });
let dmarcRecordFound;
if (dmarcRecord && Array.isArray(dmarcRecord)) {
dmarcRecordFound = domain.records.find(
(domainRecord) =>
domainRecord.type === "TXT" &&
dmarcRecord.find((dnsRecord) => dnsRecord === domainRecord.value)
);
}
if (dmarcRecordFound) {
dmarcRecordFound.status = "verified";
}

const spfRecord = await getDNSTXTRecords({ domain: domain.name });
let spfRecordFound;
if (spfRecord && Array.isArray(spfRecord)) {
spfRecordFound = domain.records.find(
(domainRecord) =>
domainRecord.type === "TXT" &&
spfRecord.find((dnsRecord) => dnsRecord === domainRecord.value)
);
}
if (spfRecordFound) {
spfRecordFound.status = "verified";
}

const verifiedDomainStatus = await getDomainVerificationStatus({ domain: domain.name });

domain.verified = verifiedDomainStatus.DkimAttributes?.[domain.name]?.DkimVerificationStatus === "Success" ? true : false;
await domain.save();

return { verified: domain.verified, domain };
Expand Down
2 changes: 1 addition & 1 deletion api/controllers/InboxController.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ export async function createInbox({
? new mongoose.Types.ObjectId(domain_id)
: undefined;
inbox.name = name;
inbox.email = email || await getNewRandomInboxEmail({ name });
inbox.email = email || `${name.replace(/[^a-zA-Z0-9]/g, "-")}@${process.env.DEFAULT_EMAIL_DOMAIN}`;
Copy link

@coderabbitai coderabbitai bot Nov 17, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

Risk of duplicate emails without uniqueness check.

The deterministic email generation can create duplicate emails when multiple inboxes share the same name. The previous getNewRandomInboxEmail implementation included a recursive uniqueness check that's now bypassed.

Additionally, names containing only non-alphanumeric characters (e.g., "!!!") would produce an invalid email format: @${DEFAULT_EMAIL_DOMAIN}.

Verify whether the Inbox schema enforces a unique constraint on the email field:

If no unique constraint exists, either:

  1. Add a unique constraint to the email field in the database schema, OR
  2. Restore the uniqueness check by keeping the random suffix approach

If a unique constraint does exist, the save() call on line 22 will throw a duplicate key error that should be handled gracefully with user-friendly error messages.

🏁 Script executed:

#!/bin/bash
# Description: Check if the Inbox schema has a unique constraint on the email field

# Search for the Inbox schema definition
ast-grep --pattern 'email: {
  $$$
}'

# Also search for unique index definitions
rg -n --type=ts -A5 -B5 'email.*unique|unique.*email' api/db/mongo/schemas/

Length of output: 410

🏁 Script executed:

#!/bin/bash
# Description: Verify getNewRandomInboxEmail usage and error handling

# Search for all calls to getNewRandomInboxEmail
echo "=== Searching for getNewRandomInboxEmail usage ==="
rg -n '\bgetNewRandomInboxEmail\b' --type=ts

# Check context around line 22 for error handling
echo "=== Checking context around line 21-22 for error handling ==="
sed -n '15,35p' api/controllers/InboxController.ts

# Search for try-catch or error handling around save operations
echo "=== Searching for error handling patterns in InboxController ==="
rg -n --type=ts -A3 '\.save\(' api/controllers/InboxController.ts

Length of output: 1309

🏁 Script executed:

#!/bin/bash
# Description: Check getNewRandomInboxEmail usage in AuthController

echo "=== AuthController usage of getNewRandomInboxEmail ==="
rg -n --type=ts -B5 -A5 'getNewRandomInboxEmail' api/controllers/AuthController.ts

Length of output: 605

🏁 Script executed:

#!/bin/bash
# Description: Search entire AuthController for getNewRandomInboxEmail function calls

echo "=== Full AuthController file ==="
cat api/controllers/AuthController.ts | grep -n "getNewRandomInboxEmail"

Length of output: 124

Error handling required for duplicate email constraint violations and invalid email format edge case.

The Inbox schema does enforce a unique constraint on the email field (confirmed at api/db/mongo/schemas/Inbox.ts:30), so duplicate emails are prevented at the database level. However, two critical issues remain:

  1. Missing error handling: The await inbox.save() call (line 22) has no try-catch, so duplicate key errors from the unique constraint will cause unhandled promise rejections.

  2. Invalid email format edge case: Names containing only non-alphanumeric characters (e.g., "!!!") produce invalid emails like @${DEFAULT_EMAIL_DOMAIN}.

Additionally, the getNewRandomInboxEmail function (lines 62-71) is now dead code and can be removed, as can its unused import in api/controllers/AuthController.ts:6.

🤖 Prompt for AI Agents 
In api/controllers/InboxController.ts around line 21 (and nearby save at line
22) the code assigns inbox.email using name which can produce an empty
local-part and then calls await inbox.save() with no error handling; add
validation to ensure the generated local-part is non-empty (e.g., strip
non-alphanumerics and if result is empty generate a safe fallback local-part or
use a random/UUID segment) before composing the email, and wrap the await
inbox.save() in a try-catch that specifically handles Mongo duplicate key errors
(E11000) by returning a controlled error response or retry logic, and
rethrow/propagate unexpected errors; additionally remove the now-dead
getNewRandomInboxEmail function from lines ~62-71 in this file and delete its
unused import from api/controllers/AuthController.ts (line 6) to eliminate dead
code and unused dependencies.

await inbox.save();
return inbox;
}
Expand Down
5 changes: 3 additions & 2 deletions api/controllers/SESController.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,12 @@ export const ses = new SESClient({
},
});

export async function verifyEmailDomain({ domain }: { domain: string }) {
export async function getDomainVerificationDkimAttributes({ domain }: { domain: string }) {
const command = new VerifyDomainDkimCommand({ Domain: domain });
return await ses.send(command);
}

export async function domainVerificationStatus({ domain }: { domain: string }) {
export async function getDomainVerificationStatus({ domain }: { domain: string }) {
return await ses.send(
new GetIdentityDkimAttributesCommand({ Identities: [domain] })
);
Expand Down Expand Up @@ -63,6 +63,7 @@ export async function sendSESMessage({
}) {
const mailOptions: any = {
from: fromName ? `"${fromName}" <${from}>` : from,
replyTo: fromName ? `"${fromName}" <${from}>` : from,
to: (to ?? []).join(", "),
cc: (cc ?? []).join(", "),
subject,
Expand Down
12 changes: 12 additions & 0 deletions api/db/mongo/schemas/Domain.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,18 @@ const domainSchema = new mongoose.Schema(
value: "inbound-smtp.us-east-2.amazonaws.com",
status: "pending",
},
{
type: "TXT",
name: "_dmarc",
value: "v=DMARC1; p=reject;",
status: "pending",
},
{
type: "TXT",
name: "@",
value: "v=spf1 include:amazonses.com ~all",
status: "pending",
},
],
},
},
Expand Down
54 changes: 53 additions & 1 deletion api/routes/v1/domains/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@ import {
getDomainsByOrganizationId,
verifyDomainDNS,
} from "../../../controllers/DomainController";
import {
getDomainVerificationDkimAttributes,
getDomainVerificationStatus,
} from "../../../controllers/SESController";
Comment on lines +12 to +15
Copy link

@coderabbitai coderabbitai bot Nov 17, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

Verify if getDomainVerificationStatus is used.

The import includes getDomainVerificationStatus but it doesn't appear to be used in this file. Only getDomainVerificationDkimAttributes is used (Line 106).

Run this script to verify the usage:

🏁 Script executed:

#!/bin/bash
# Description: Check if getDomainVerificationStatus is used in this routes file

# Search for usage of getDomainVerificationStatus in the file
rg -n 'getDomainVerificationStatus' api/routes/v1/domains/index.ts

Length of output: 33


---

🏁 Script executed:

```shell
#!/bin/bash
# Check if getDomainVerificationStatus is re-exported from this file
rg -n 'export.*getDomainVerificationStatus' api/routes/v1/domains/index.ts

# Check if other files import getDomainVerificationStatus from this routes file
rg -n "from.*api/routes/v1/domains" --type=ts | head -20

Length of output: 0


---

🏁 Script executed:

```shell
#!/bin/bash
# Get the full content of the file to verify getDomainVerificationStatus usage
cat api/routes/v1/domains/index.ts | grep -i 'getDomainVerificationStatus\|export'

Length of output: 53

Remove the unused getDomainVerificationStatus import.

Verification confirms that getDomainVerificationStatus is imported but never used in this file. Only getDomainVerificationDkimAttributes is actively used. Remove the unused import from lines 12-15.

🤖 Prompt for AI Agents 
In api/routes/v1/domains/index.ts around lines 12 to 15, the import list
includes getDomainVerificationStatus which is not used in this file; remove
getDomainVerificationStatus from the import statement so only
getDomainVerificationDkimAttributes is imported, then save the file and run a
quick lint/type check to confirm no unused-import warnings remain.


const router = Router({ mergeParams: true });

Expand All @@ -28,7 +32,9 @@ router.post(
.isString()
.trim()
.notEmpty()
.matches(/^(?:[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?\.)+[a-zA-Z]{2,}$/)
.matches(
/^(?:[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?\.)+[a-zA-Z]{2,}$/
)
.withMessage("Invalid domain name format"),
expressValidatorMiddleware,
async (
Expand Down Expand Up @@ -82,6 +88,52 @@ router.post(
}
);

router.get(
"/:domainId/dns",
async (
req: Request<{ organizationId: string; domainId: string }, {}, {}>,
res: Response
) => {
const domain = await getDomainByOrganizationIdAndName({
organizationId: req.organization._id.toString(),
name: req.params.domainId,
});
if (!domain) {
return res.status(404).json({ error: "Domain not found" });
}

const verifiedDomainDkimAttributes =
await getDomainVerificationDkimAttributes({ domain: domain.name });

if (!verifiedDomainDkimAttributes.DkimTokens) {
return res.status(400).json({ error: "Failed to get DKIM attributes" });
}

return res.json([
{
type: "MX",
name: "@",
value: "inbound-smtp.us-east-2.amazonaws.com",
},
{
type: "TXT",
name: "@",
value: "v=spf1 include:amazonses.com ~all",
},
{
type: "TXT",
name: "_dmarc",
value: "v=DMARC1; p=reject;",
},
Comment on lines +112 to +127
Copy link

@coderabbitai coderabbitai bot Nov 17, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Critical: Hardcoded AWS region and email policies.

Several hardcoded values create deployment and configuration concerns:

  1. AWS Region (Line 116): The MX record hardcodes us-east-2, which will break email delivery if the application is deployed in a different AWS region or needs to support multiple regions.

  2. Email Security Policies: The SPF and DMARC policies are hardcoded with specific settings that may not suit all organizations:

    • SPF ~all (softfail) vs -all (hardfail)
    • DMARC p=reject is the strictest policy

Consider extracting these values to environment variables or configuration:

+ // In a config file or from environment variables
+ const AWS_SES_REGION = process.env.AWS_SES_REGION || 'us-east-2';
+ const SPF_POLICY = process.env.SPF_POLICY || 'v=spf1 include:amazonses.com ~all';
+ const DMARC_POLICY = process.env.DMARC_POLICY || 'v=DMARC1; p=reject;';

  return res.json([
    {
      type: "MX",
      name: "@",
-     value: "inbound-smtp.us-east-2.amazonaws.com",
+     value: `inbound-smtp.${AWS_SES_REGION}.amazonaws.com`,
    },
    {
      type: "TXT",
      name: "@",
-     value: "v=spf1 include:amazonses.com ~all",
+     value: SPF_POLICY,
    },
    {
      type: "TXT",
      name: "_dmarc",
-     value: "v=DMARC1; p=reject;",
+     value: DMARC_POLICY,
    },
📝 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
return res.json([
{
type: "MX",
name: "@",
value: "inbound-smtp.us-east-2.amazonaws.com",
},
{
type: "TXT",
name: "@",
value: "v=spf1 include:amazonses.com ~all",
},
{
type: "TXT",
name: "_dmarc",
value: "v=DMARC1; p=reject;",
},
// In a config file or from environment variables
const AWS_SES_REGION = process.env.AWS_SES_REGION || 'us-east-2';
const SPF_POLICY = process.env.SPF_POLICY || 'v=spf1 include:amazonses.com ~all';
const DMARC_POLICY = process.env.DMARC_POLICY || 'v=DMARC1; p=reject;';
return res.json([
{
type: "MX",
name: "@",
value: `inbound-smtp.${AWS_SES_REGION}.amazonaws.com`,
},
{
type: "TXT",
name: "@",
value: SPF_POLICY,
},
{
type: "TXT",
name: "_dmarc",
value: DMARC_POLICY,
},
🤖 Prompt for AI Agents 
In api/routes/v1/domains/index.ts around lines 112 to 127, the MX host and
SPF/DMARC strings are hardcoded (us-east-2, v=spf1 ... ~all, v=DMARC1;
p=reject;); change this to read AWS_REGION, SPF_POLICY and DMARC_POLICY (or a
config object) from environment/config with sensible defaults and validation,
construct the MX value dynamically as `inbound-smtp.{AWS_REGION}.amazonaws.com`
using the region value, and replace the hardcoded SPF/DMARC strings with the
env-driven values so deploys in other regions or with different security
policies work correctly (include fallbacks and a brief error/log if required env
vars are missing or invalid).

...verifiedDomainDkimAttributes.DkimTokens.map((token) => ({
type: "CNAME",
name: `${token}_domainkey.${domain.name}`,
value: `${token}.dkim.amazonses.com`,
}))
]);
}
);

router.delete(
"/:domainId",
async (
Expand Down
75 changes: 0 additions & 75 deletions app/README.md
View file
Open in desktop

This file was deleted.

Loading