Skip to content

fix(apps/ensadmin): validate name query params on name details page #1834

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

Draft
notrab wants to merge 24 commits into
base: main
Choose a base branch
from
Draft

fix(apps/ensadmin): validate name query params on name details page #1834

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
4bf234f
first pass
notrab Mar 26, 2026
f7893a2
use ErrorInfo component
notrab Mar 26, 2026
569f803
docs(changeset): Validate name query param as InterpretedName and Nor…
notrab Mar 26, 2026
255190e
handle null name
notrab Mar 26, 2026
0a17eea
update the error message
notrab Mar 26, 2026
0ab0331
form validation
notrab Mar 27, 2026
14e162f
fix url state and empty params
notrab Mar 28, 2026
72e310e
Update apps/ensadmin/src/app/name/page.tsx
notrab Mar 28, 2026
1f29be9
Merge branch 'main' into notrab/normalized-name-state
notrab Mar 28, 2026
a205294
tidy normalize once logic
notrab Mar 28, 2026
1fd70f1
update normalization logic for react params
notrab Mar 28, 2026
2ff21a5
startover
notrab Mar 29, 2026
3c05ce3
fix conflicts
notrab Mar 29, 2026
bb498a4
formatInterpretedNameForDisplay
notrab Mar 29, 2026
077628b
make interpretedName more explicit
notrab Mar 29, 2026
3308ed9
Merge branch 'main' into notrab/normalized-name-state
notrab Mar 30, 2026
e92eb03
Merge branch 'main' into notrab/normalized-name-state
notrab Mar 31, 2026
310919a
Apply suggestion from @notrab
notrab Mar 31, 2026
f2c0398
Merge branch 'main' into notrab/normalized-name-state
notrab Mar 31, 2026
32ac25e
Merge remote-tracking branch 'origin/main' into notrab/normalized-nam…
notrab May 7, 2026
8663711
apply previous changes
notrab May 7, 2026
456c3e5
lint
notrab May 7, 2026
298885e
apply stashed fix
notrab May 7, 2026
d1cd779
update changeset
notrab May 7, 2026
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
5 changes: 5 additions & 0 deletions .changeset/spicy-gifts-say.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"ensadmin": patch
---

Validate name query param as InterpretedName and NormalizedName before rendering the name details page, displaying appropriate error states for invalid or unsupported names.
9 changes: 6 additions & 3 deletions apps/ensadmin/src/app/@actions/name/page.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import { getEnsManagerNameDetailsUrl } from "@namehash/namehash-ui";
import { useSearchParams } from "next/navigation";

import type { Name } from "@ensnode/ensnode-sdk";
import { isNormalizedName, type Name } from "@ensnode/ensnode-sdk";

import { ExternalLinkWithIcon } from "@/components/link";
import { Button } from "@/components/ui/button";
Expand All @@ -13,11 +13,14 @@ export default function ActionsNamePage() {
const searchParams = useSearchParams();
const nameParam = searchParams.get("name");
Copy link
Copy Markdown
Member

@lightwalker-eth lightwalker-eth Mar 26, 2026

Choose a reason for hiding this comment

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

Let's please move the typing for things to where they originate. Ex: If this is a Name and not an arbitrary string, then we should apply that typing here.

Please apply this idea everywhere it is relevant.

Note how the reinterpretation of this value as a Name is happening several lines below. I don't like that.

Copy link
Copy Markdown
Member Author

@notrab notrab Mar 28, 2026

Choose a reason for hiding this comment

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

Alright, I've changed this to const nameFromQuery = searchParams.get("name") as Name | null which is really the only other option here as far as I can tell.


const name = nameParam ? (decodeURIComponent(nameParam) as Name) : null;
const name = nameParam ? (nameParam as Name) : null;

const { data: namespace } = useNamespace();

const ensAppProfileUrl = name && namespace ? getEnsManagerNameDetailsUrl(name, namespace) : null;
const ensAppProfileUrl =
name && isNormalizedName(name) && namespace
? getEnsManagerNameDetailsUrl(name, namespace)
Copy link
Copy Markdown
Member

@lightwalker-eth lightwalker-eth Mar 26, 2026

Choose a reason for hiding this comment

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

Any special reason not to move the isNormalizedName check into the implementation of getEnsManagerNameDetailsUrl?

Please ignore if not relevant.

Copy link
Copy Markdown
Member Author

@notrab notrab Mar 28, 2026

Choose a reason for hiding this comment

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

getEnsManagerNameDetailsUrl lives in the namehash-ui package. isNormalizedName is the responsibility of the actions here. Maybe we should not link at all if it's not normalized.

Copy link
Copy Markdown
Member

@lightwalker-eth lightwalker-eth Mar 29, 2026

Choose a reason for hiding this comment

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

I don't understand why you mention how getEnsManagerNameDetailsUrl lives in the namehash-ui package? How is that relevant to the ideas here?

If we say that we don't want to link to the ENS Manager Name Details page for unnormalized names, then that decision should move into getEnsManagerNameDetailsUrl so that we never need to talk about it again.

Otherwise we have to keep endlessly talking about the implementation details of unnormalized name handling which I really hate as I've been doing that over and over again for 4 years.

: null;

if (!ensAppProfileUrl) return null;

Expand Down
6 changes: 3 additions & 3 deletions apps/ensadmin/src/app/@breadcrumbs/name/page.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import { NameDisplay } from "@namehash/namehash-ui";
import { useSearchParams } from "next/navigation";

import type { Name } from "@ensnode/ensnode-sdk";
import { isInterpretedName, type Name } from "@ensnode/ensnode-sdk";

import BreadcrumbsGroup from "@/components/breadcrumbs/group";
import {
Expand All @@ -20,7 +20,7 @@ export default function Page() {
const { retainCurrentRawConnectionUrlParam } = useRawConnectionUrlParam();
const exploreNamesBaseHref = retainCurrentRawConnectionUrlParam("/name");

const name = nameParam ? (decodeURIComponent(nameParam) as Name) : null;
const name = nameParam ? (nameParam as Name) : null;

return (
<BreadcrumbsGroup name="ENS Explorer">
Expand All @@ -32,7 +32,7 @@ export default function Page() {
<BreadcrumbSeparator className="hidden md:block" />
<BreadcrumbItem>
<BreadcrumbPage>
<NameDisplay name={name} />
{isInterpretedName(name) ? <NameDisplay name={name} /> : name}
Comment thread
notrab marked this conversation as resolved.
Outdated
</BreadcrumbPage>
</BreadcrumbItem>
</>
Expand Down
4 changes: 2 additions & 2 deletions apps/ensadmin/src/app/name/_components/NameDetailPageContent.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
"use client";

import { ASSUME_IMMUTABLE_QUERY, useRecords } from "@ensnode/ensnode-react";
import { type Name, type ResolverRecordsSelection } from "@ensnode/ensnode-sdk";
import { type NormalizedName, type ResolverRecordsSelection } from "@ensnode/ensnode-sdk";

import { Card, CardContent } from "@/components/ui/card";
import { useActiveNamespace } from "@/hooks/active/use-active-namespace";
Expand Down Expand Up @@ -39,7 +39,7 @@ const AllRequestedTextRecords = [
];

interface NameDetailPageContentProps {
name: Name;
name: NormalizedName;
Copy link
Copy Markdown
Member Author

@notrab notrab Mar 29, 2026

Choose a reason for hiding this comment

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

The original issue stated:

Refine typing of "downstream" name variables, ex: in ProfileHeader

and most recently:

As much as possible, we need to stop passing Name values around anywhere because they are a motherfucker of problems

}

export function NameDetailPageContent({ name }: NameDetailPageContentProps) {
Expand Down
29 changes: 29 additions & 0 deletions apps/ensadmin/src/app/name/_components/NameErrors.tsx
View file
Open in desktop
Copy link
Copy Markdown
Member Author

@notrab notrab Mar 29, 2026

Choose a reason for hiding this comment

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

I'm aware this is a new file, and not sure where this lives long term.

Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import type { Name } from "@ensnode/ensnode-sdk";

import { ErrorInfo } from "@/components/error-info";

interface NameErrorProps {
name: Name;
}

export function InvalidNameError({ name }: NameErrorProps) {
return (
<section className="p-6">
<ErrorInfo
title="Invalid Name"
description={`The provided name "${name}" is not a valid ENS name.`}
Comment thread
lightwalker-eth marked this conversation as resolved.
Outdated
Copy link
Copy Markdown
Member

@lightwalker-eth lightwalker-eth Mar 29, 2026

Choose a reason for hiding this comment

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

We should never take raw name values and put them in UIs. All names appearing in UIs should go through the logic we define in the NameDisplay component.

Ugh.. I see that we need to create another new utility function for this but I need to rush to my gate and not miss my flight.

If we need to display names within a formatted string (rather than directly through a UI component such as NameDisplay) then suggest to write a new utility function inside packages/ensnode-sdk/src/ens/names.ts.

I'm just drafting this in the GitHub PR feedback comment box. Please carefully review and unit test this.

For example, I'm worried about the handling of inputs such as "" (empty string).

NOTE: This function only takes an InterpretedName and not a Name.

export const formatInterpretedNameForDisplay = (name: InterpretedName): Name => {
  const displayLabels = name.split(".").map((label: Label) => {
    if (isNormalizedLabel(label)) {
      // all normalized labels should be displayed in beautified form
      // NOTE! This may return labels that are not normalized. Therefore this function returns a `Name` and not an `InterpretedName`.
      return ens_beautify(label);
    } else {
      // since `name` is an InterpretedName, this label must therefore be an encoded labelhash, so simply "normalize" it to lowercase
      return label.toLowerCase();
    }
  });
  return displayLabels.join(".");
};

Copy link
Copy Markdown
Member Author

@notrab notrab Mar 29, 2026

Choose a reason for hiding this comment

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

@lightwalker-eth noted. I note that this also should live in packages/ensnode-sdk/src/ens/names.ts.

Copy link
Copy Markdown
Member Author

@notrab notrab Mar 29, 2026

Choose a reason for hiding this comment

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

I've now moved it, but I renamed the name arg to interpretedName thinking you'd prefer this to be more explicit. It's clearer when calling it without readingt he JSDoc what this arg is. Let me know if you disagree,.

/>
</section>
);
}

export function EncodedLabelhashUnsupportedError({ name }: NameErrorProps) {
return (
<section className="p-6">
<ErrorInfo
title="Encoded Labelhash Detected"
description={`The provided name "${name}" contains encoded labelhashes. Support for resolving names with encoded labelhashes is in progress and coming soon.`}
Copy link
Copy Markdown
Member

@lightwalker-eth lightwalker-eth Mar 29, 2026

Choose a reason for hiding this comment

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

We should never take raw name values and put them in UIs. All names appearing in UIs should go through the logic we define in the NameDisplay component.

/>
</section>
);
}
Comment on lines +11 to +20
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Mar 29, 2026

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Consider displaying the interpreted name in the error message.

Since InterpretedNameUnsupportedError is shown for valid InterpretedName values (with encoded labelhashes), you could accept a name: InterpretedName prop and display it using formatInterpretedNameForDisplay for better user context. This aligns with past review feedback suggesting <InterpretedNameUnsupportedError name={...} />.

💡 Optional enhancement 
+import type { InterpretedName } from "@ensnode/ensnode-sdk";
+import { formatInterpretedNameForDisplay } from "@/lib/format-interpreted-name-for-display";
 import { ErrorInfo } from "@/components/error-info";

-export function InterpretedNameUnsupportedError() {
+export function InterpretedNameUnsupportedError({ name }: { name: InterpretedName }) {
   return (
     <section className="p-6">
       <ErrorInfo
         title="Encoded Labelhash Detected"
-        description="The provided name contains encoded labelhashes. Support for resolving names with encoded labelhashes is in progress and coming soon."
+        description={`The name "${formatInterpretedNameForDisplay(name)}" contains encoded labelhashes. Support for resolving names with encoded labelhashes is in progress and coming soon.`}
       />
     </section>
   );
 }
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@apps/ensadmin/src/app/name/_components/NameErrors.tsx` around lines 11 - 20,
Update the InterpretedNameUnsupportedError component to accept a prop name of
type InterpretedName and show the interpreted name in the error description
using formatInterpretedNameForDisplay; specifically, change the
InterpretedNameUnsupportedError signature to take { name: InterpretedName },
call formatInterpretedNameForDisplay(name) and include that string in the
ErrorInfo description (or add a separate field) so the rendered message shows
the actual interpreted name for context.

4 changes: 2 additions & 2 deletions apps/ensadmin/src/app/name/_components/ProfileHeader.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,14 @@

import { EnsAvatar, NameDisplay } from "@namehash/namehash-ui";

import type { ENSNamespaceId, Name } from "@ensnode/ensnode-sdk";
import type { ENSNamespaceId, NormalizedName } from "@ensnode/ensnode-sdk";

import { ExternalLinkWithIcon } from "@/components/link";
import { Card, CardContent } from "@/components/ui/card";
import { beautifyUrl } from "@/lib/beautify-url";

interface ProfileHeaderProps {
name: Name;
name: NormalizedName;
Copy link
Copy Markdown
Member Author

@notrab notrab Mar 29, 2026

Choose a reason for hiding this comment

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

This was changed because of the comments made about the downstream handling, and:

As much as possible, we need to stop passing Name values around anywhere because they are a motherfucker of problems

NameDetailPageContent only ever receives a validated normalized name from the detail page check (isNormalizedName(nameFromQuery)), so the prop type was tightened to reflect that guarantee

namespaceId: ENSNamespaceId;
headerImage?: string | null;
websiteUrl?: string | null;
Expand Down
13 changes: 12 additions & 1 deletion apps/ensadmin/src/app/name/page.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import { type ChangeEvent, useMemo, useState } from "react";
import { ENSNamespaceIds } from "@ensnode/datasources";
import {
getNamespaceSpecificValue,
isInterpretedName,
isNormalizedName,
type Name,
type NamespaceSpecificValue,
} from "@ensnode/ensnode-sdk";
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
Expand All @@ -19,6 +21,7 @@ import { useActiveNamespace } from "@/hooks/active/use-active-namespace";
import { useRawConnectionUrlParam } from "@/hooks/use-connection-url-param";

import { NameDetailPageContent } from "./_components/NameDetailPageContent";
import { EncodedLabelhashUnsupportedError, InvalidNameError } from "./_components/NameErrors";

const EXAMPLE_NAMES: NamespaceSpecificValue<string[]> = {
Comment thread
notrab marked this conversation as resolved.
Outdated
default: [
Expand Down Expand Up @@ -90,7 +93,15 @@ export default function ExploreNamesPage() {
setRawInputName(e.target.value);
};

if (nameFromQuery) {
if (nameFromQuery !== null) {
if (!isInterpretedName(nameFromQuery)) {
return <InvalidNameError name={nameFromQuery} />;
}

if (!isNormalizedName(nameFromQuery)) {
return <EncodedLabelhashUnsupportedError name={nameFromQuery} />;
}
Comment thread
notrab marked this conversation as resolved.

return <NameDetailPageContent name={nameFromQuery} />;
}

Expand Down
Loading