-
Notifications
You must be signed in to change notification settings - Fork 16
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
4bf234f
f7893a2
569f803
255190e
0a17eea
0ab0331
14e162f
72e310e
1f29be9
a205294
1fd70f1
2ff21a5
3c05ce3
bb498a4
077628b
3308ed9
e92eb03
310919a
f2c0398
32ac25e
8663711
456c3e5
298885e
d1cd779
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| --- | ||
| "ensadmin": patch | ||
| --- | ||
|
|
||
| Validate name input on the Explore Names page (`/name`). The form normalizes user input before navigating and displays inline errors for unnormalizable names or names with encoded labelhashes (resolution support is in progress). Query params on the detail page are validated against `InterpretedName`. | ||
|
|
||
| Unnormalized names and names with encoded labelhashes each show a dedicated error instead of falling through to a broken detail page. | ||
|
|
||
| An empty `?name=` shows the form rather than the detail page. |
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,20 @@ | ||||||||||||
| import { ErrorInfo } from "@/components/error-info"; | ||||||||||||
|
|
||||||||||||
| export function UnnormalizedNameError() { | ||||||||||||
| return ( | ||||||||||||
| <section className="p-6"> | ||||||||||||
| <ErrorInfo title="Invalid Name" description="The provided name is not a valid ENS name." /> | ||||||||||||
|
||||||||||||
| <ErrorInfo title="Invalid Name" description="The provided name is not a valid ENS name." /> | |
| <ErrorInfo | |
| title="Unnormalized Name" | |
| description="The provided name is not in normalized ENS form. Please use a normalized ENS name (for example, lowercase and properly formatted)." | |
| /> |
There was a problem hiding this comment.
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.
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
| @@ -1,7 +1,15 @@ | ||||
| "use client"; | ||||
|
|
||||
| import { NameDisplay } from "@namehash/namehash-ui"; | ||||
| import type { Name } from "enssdk"; | ||||
| import { | ||||
| asInterpretedName, | ||||
| asLiteralName, | ||||
| type InterpretedName, | ||||
| isInterpretedName, | ||||
| isNormalizedName, | ||||
| literalNameToInterpretedName, | ||||
| type Name, | ||||
| } from "enssdk"; | ||||
| import { useRouter, useSearchParams } from "next/navigation"; | ||||
| import { type ChangeEvent, useMemo, useState } from "react"; | ||||
|
|
||||
|
|
@@ -16,8 +24,9 @@ import { useActiveEnsNodeStackInfo } from "@/hooks/active/use-active-ensnode-sta | |||
| import { useRawConnectionUrlParam } from "@/hooks/use-connection-url-param"; | ||||
|
|
||||
| import { NameDetailPageContent } from "./_components/NameDetailPageContent"; | ||||
| import { InterpretedNameUnsupportedError, UnnormalizedNameError } from "./_components/NameErrors"; | ||||
|
|
||||
| const EXAMPLE_NAMES: NamespaceSpecificValue<string[]> = { | ||||
| const EXAMPLE_NAMES: NamespaceSpecificValue<InterpretedName[]> = { | ||||
| default: [ | ||||
| "vitalik.eth", | ||||
| "gregskril.eth", | ||||
|
|
@@ -31,15 +40,15 @@ const EXAMPLE_NAMES: NamespaceSpecificValue<string[]> = { | |||
| "lens.xyz", | ||||
| "brantly.eth", | ||||
| "lightwalker.eth", | ||||
| ], | ||||
| ].map(asInterpretedName), | ||||
| [ENSNamespaceIds.Sepolia]: [ | ||||
| "gregskril.eth", | ||||
| "vitalik.eth", | ||||
| "myens.eth", | ||||
| "recordstest.eth", | ||||
| "arrondesean.eth", | ||||
| "decode.eth", | ||||
| ], | ||||
| ].map(asInterpretedName), | ||||
| [ENSNamespaceIds.EnsTestEnv]: [ | ||||
| "alias.eth", | ||||
| "changerole.eth", | ||||
|
|
@@ -53,14 +62,15 @@ const EXAMPLE_NAMES: NamespaceSpecificValue<string[]> = { | |||
| "sub2.parent.eth", | ||||
| "test.eth", | ||||
| "wallet.linked.parent.eth", | ||||
| ], | ||||
| ].map(asInterpretedName), | ||||
| }; | ||||
|
|
||||
| export default function ExploreNamesPage() { | ||||
| const router = useRouter(); | ||||
| const searchParams = useSearchParams(); | ||||
| const nameFromQuery = searchParams.get("name"); | ||||
| const [rawInputName, setRawInputName] = useState<Name>(""); | ||||
| const [formError, setFormError] = useState<string | null>(null); | ||||
|
|
||||
| const { namespace } = useActiveEnsNodeStackInfo().ensIndexer; | ||||
| const exampleNames = useMemo( | ||||
|
|
@@ -72,22 +82,49 @@ export default function ExploreNamesPage() { | |||
|
|
||||
| const handleSubmit = (e: React.FormEvent<HTMLFormElement>) => { | ||||
| e.preventDefault(); | ||||
|
|
||||
| // TODO: Input validation and normalization. | ||||
| // see: https://github.com/namehash/ensnode/issues/1140 | ||||
|
|
||||
| const href = retainCurrentRawConnectionUrlParam(getNameDetailsRelativePath(rawInputName)); | ||||
|
|
||||
| setFormError(null); | ||||
|
|
||||
| if (rawInputName.trim() === "") return; | ||||
|
|
||||
| let interpreted: InterpretedName; | ||||
| try { | ||||
| // Allow encoded labelhashes through; throw on unnormalizable labels. | ||||
| interpreted = literalNameToInterpretedName(asLiteralName(rawInputName), { | ||||
|
Comment on lines
+87
to
+92
|
||||
| allowEncodedLabelHashes: true, | ||||
| }); | ||||
| } catch { | ||||
| setFormError("The provided input is not a valid ENS name."); | ||||
| return; | ||||
| } | ||||
|
|
||||
| if (!isNormalizedName(interpreted)) { | ||||
| setFormError( | ||||
| "The provided input contains encoded labelhashes. Support for resolving names with encoded labelhashes is in progress and coming soon.", | ||||
| ); | ||||
| return; | ||||
| } | ||||
|
|
||||
| const href = retainCurrentRawConnectionUrlParam(getNameDetailsRelativePath(interpreted)); | ||||
| router.push(href); | ||||
| }; | ||||
|
|
||||
| const handleRawInputNameChange = (e: ChangeEvent<HTMLInputElement>) => { | ||||
| e.preventDefault(); | ||||
|
|
||||
| setFormError(null); | ||||
| setRawInputName(e.target.value); | ||||
| }; | ||||
|
|
||||
| if (nameFromQuery) { | ||||
| // Detail page: validate name from query params using only validation checks (no normalization). | ||||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||
| // see: https://github.com/namehash/ensnode/issues/1140 | ||||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Done, I can remove these TODos now. |
||||
| if (nameFromQuery !== null && nameFromQuery !== "") { | ||||
| if (!isInterpretedName(nameFromQuery)) { | ||||
| return <UnnormalizedNameError />; | ||||
| } | ||||
|
|
||||
| if (!isNormalizedName(nameFromQuery)) { | ||||
| return <InterpretedNameUnsupportedError />; | ||||
| } | ||||
|
|
||||
| return <NameDetailPageContent name={nameFromQuery} />; | ||||
| } | ||||
|
|
||||
|
|
@@ -112,12 +149,13 @@ export default function ExploreNamesPage() { | |||
| /> | ||||
| <Button | ||||
| type="submit" | ||||
| disabled={rawInputName.length === 0} | ||||
| disabled={rawInputName.trim().length === 0} | ||||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @lightwalker-eth I added
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @notrab In my mind we make "Empty" a special class of the interpretation outcome and then build logic such as disabling a "View Profile" button based on the "Empty" class of interpretation outcomes. |
||||
| className="max-sm:self-stretch" | ||||
| > | ||||
| View Profile | ||||
| </Button> | ||||
| </fieldset> | ||||
| {formError && <p className="text-sm text-red-600">{formError}</p>} | ||||
| </form> | ||||
| <div className="flex flex-col gap-2 justify-center"> | ||||
| <p className="text-sm font-medium leading-none">Examples:</p> | ||||
|
|
||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,5 +1,5 @@ | ||||||||||||||||||||
| import type { Name } from "enssdk"; | ||||||||||||||||||||
| import { beautifyName } from "enssdk"; | ||||||||||||||||||||
| import { beautifyInterpretedName, isInterpretedName } from "enssdk"; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| interface NameDisplayProps { | ||||||||||||||||||||
| name: Name; | ||||||||||||||||||||
|
|
@@ -9,10 +9,16 @@ interface NameDisplayProps { | |||||||||||||||||||
| /** | ||||||||||||||||||||
| * Displays an ENS name in beautified form. | ||||||||||||||||||||
| * | ||||||||||||||||||||
| * @param name - The name to display in beautified form. | ||||||||||||||||||||
| * If the provided name is not a valid InterpretedName, displays | ||||||||||||||||||||
| * "(invalid name)" instead. | ||||||||||||||||||||
| * | ||||||||||||||||||||
| * @param name - The name to display. | ||||||||||||||||||||
| */ | ||||||||||||||||||||
| export function NameDisplay({ name, className = "nhui:font-medium" }: NameDisplayProps) { | ||||||||||||||||||||
| const beautifiedName = beautifyName(name); | ||||||||||||||||||||
| if (!isInterpretedName(name)) { | ||||||||||||||||||||
| return <span className={className}>(invalid name)</span>; | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
Comment on lines
+18
to
+20
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Verify whether NameDisplay is used with names that are not pre-validated as InterpretedName.
# 1) Locate NameDisplay imports and JSX call sites.
rg -n -C3 --type=ts --type=tsx 'NameDisplay'
rg -n -C3 --type=tsx '<NameDisplay[^>]*\bname='
# 2) In files using NameDisplay, inspect nearby guards/typing for interpreted-name validation.
rg -n -C4 --type=ts --type=tsx 'isInterpretedName\(|isNormalizedName\(|:\s*Name\b|:\s*InterpretedName\b|:\s*NormalizedName\b'Repository: namehash/ensnode Length of output: 143 🏁 Script executed: #!/bin/bash
# Verify NameDisplay usage with corrected rg syntax
# 1) Search for NameDisplay imports and usages (using basic ts type or combined search)
rg -n 'NameDisplay' --type=ts
# 2) Search for JSX call sites of NameDisplay
rg -n '<NameDisplay' --type=ts
# 3) Search for isInterpretedName, isNormalizedName, Name type, InterpretedName type usage
rg -n 'isInterpretedName|isNormalizedName|:\s*Name\b|:\s*InterpretedName' --type=tsRepository: namehash/ensnode Length of output: 13337 🏁 Script executed: #!/bin/bash
# 1. Check what namedRegistrarAction.name is in RegistrarActionCard
echo "=== RegistrarActionCard context ==="
sed -n '295,310p' packages/namehash-ui/src/components/registrar-actions/RegistrarActionCard.tsx
# 2. Check ResolveAndDisplayIdentity and what name it passes
echo ""
echo "=== ResolveAndDisplayIdentity context ==="
sed -n '170,190p' packages/namehash-ui/src/components/identity/ResolveAndDisplayIdentity.tsx
# 3. Check the exampleName in ensadmin/app/name/page.tsx
echo ""
echo "=== ensadmin name/page.tsx context ==="
sed -n '165,180p' apps/ensadmin/src/app/name/page.tsx
# 4. Check ProfileHeader context
echo ""
echo "=== ProfileHeader context ==="
sed -n '70,85p' apps/ensadmin/src/app/name/_components/ProfileHeader.tsx
# 5. Check beautifyName implementation
echo ""
echo "=== beautifyName function ==="
rg -A10 'export const beautifyName' packages/ensnode-sdk/src/ens/names.tsRepository: namehash/ensnode Length of output: 3099 🏁 Script executed: #!/bin/bash
# 1. Check namedRegistrarAction type in RegistrarActionCard
echo "=== RegistrarActionCard namedRegistrarAction type ==="
sed -n '1,50p' packages/namehash-ui/src/components/registrar-actions/RegistrarActionCard.tsx | grep -A5 "namedRegistrarAction\|RegistrarAction"
# 2. Check the registrar-actions types
echo ""
echo "=== Check registrar-actions types ==="
rg -n 'namedRegistrarAction\s*:' packages/namehash-ui/src/components/registrar-actions/RegistrarActionCard.tsx -A2
# 3. Check identity type in ResolveAndDisplayIdentity
echo ""
echo "=== ResolveAndDisplayIdentity identity type ==="
sed -n '1,100p' packages/namehash-ui/src/components/identity/ResolveAndDisplayIdentity.tsx | grep -A5 'identity\|Identity'
# 4. Check ensadmin ProfileHeader name type
echo ""
echo "=== ProfileHeader name parameter type ==="
sed -n '1,30p' apps/ensadmin/src/app/name/_components/ProfileHeader.tsx
# 5. Check EXAMPLE_ADDRESSES type
echo ""
echo "=== EXAMPLE_ADDRESSES definition ==="
sed -n '40,50p' apps/ensadmin/src/app/inspect/_lib/example-addresses.tsRepository: namehash/ensnode Length of output: 5195 🏁 Script executed: #!/bin/bash
# 1. Check NormalizedName type definition
echo "=== NormalizedName definition ==="
rg -n 'type NormalizedName\|export.*NormalizedName' packages/ensnode-sdk/src/ -A3
# 2. Check InterpretedName definition
echo ""
echo "=== InterpretedName definition ==="
rg -n 'type InterpretedName\|export.*InterpretedName' packages/ensnode-sdk/src/ -A3
# 3. Check NamedRegistrarAction type
echo ""
echo "=== NamedRegistrarAction definition ==="
rg -n 'NamedRegistrarAction' packages/ensnode-sdk/src/ -B2 -A8 | head -40
# 4. Check Identity.name type
echo ""
echo "=== Identity type definition ==="
rg -n 'interface Identity|type Identity' packages/ensnode-sdk/src/ -A15Repository: namehash/ensnode Length of output: 5789 🏁 Script executed: #!/bin/bash
# Search for type aliases and exports
echo "=== Search for NormalizedName and InterpretedName in all files ==="
rg -n 'NormalizedName|InterpretedName' packages/ensnode-sdk/src/ens/ --type=ts -B2 -A2 | head -100
# Check the main ens directory structure
echo ""
echo "=== Files in ENS directory ==="
fd '\.ts$' packages/ensnode-sdk/src/ens/
# Check what beautifyName actually accepts and what isInterpretedName checks
echo ""
echo "=== isInterpretedName implementation ==="
sed -n '100,120p' packages/ensnode-sdk/src/shared/interpretation/interpreted-names-and-labels.tsRepository: namehash/ensnode Length of output: 8919 🏁 Script executed: #!/bin/bash
# Check InterpretedLabel definition
echo "=== InterpretedLabel definition ==="
rg -n 'type InterpretedLabel|InterpretedLabel =' packages/ensnode-sdk/src/ -B2 -A2
# Check isInterpetedLabel implementation (note the typo in the code)
echo ""
echo "=== isInterpetedLabel implementation ==="
rg -n 'function isInterpetedLabel|export.*isInterpetedLabel' packages/ensnode-sdk/src/ -A10
# Check relationship between NormalizedLabel and InterpretedLabel
echo ""
echo "=== Check ens/types.ts for label definitions ==="
sed -n '1,150p' packages/ensnode-sdk/src/ens/types.ts | tail -60Repository: namehash/ensnode Length of output: 6391 🏁 Script executed: #!/bin/bash
# Get the full NamedRegistrarAction interface definition
echo "=== NamedRegistrarAction full definition ==="
sed -n '32,46p' packages/ensnode-sdk/src/ensapi/api/registrar-actions/response.ts
# Get Identity type definition with name field
echo ""
echo "=== Identity and ResolvedIdentity types ==="
sed -n '140,170p' packages/ensnode-sdk/src/identity/types.ts
# Check if NormalizedName passes isInterpretedName test
echo ""
echo "=== Test: Does NormalizedName pass isInterpretedName? ==="
rg -n 'NormalizedName.*isInterpretedName|isInterpretedName.*NormalizedName' packages/ensnode-sdk/src/ -B2 -A2Repository: namehash/ensnode Length of output: 833 🏁 Script executed: #!/bin/bash
# Get the full identity types
echo "=== Full Identity type definitions ==="
sed -n '50,160p' packages/ensnode-sdk/src/identity/types.ts
# Test whether NormalizedName is accepted as InterpretedName
echo ""
echo "=== Can NormalizedName be used where InterpretedName is required? ==="
grep -r 'NormalizedName' packages/ensnode-sdk/src/identity/types.tsRepository: namehash/ensnode Length of output: 2617 🏁 Script executed: #!/bin/bash
# Check what types are passed to NameDisplay in reality
echo "=== Trace ResolveAndDisplayIdentity.NamedIdentity.name ==="
rg -n 'NamedIdentity\|name: Name' packages/ensnode-sdk/src/identity/types.ts -B5 -A5
# Check if there's a test showing NormalizedName passing isInterpretedName
echo ""
echo "=== isInterpretedName tests ==="
sed -n '1,200p' packages/ensnode-sdk/src/shared/interpretation/interpreted-names-and-labels.test.ts | grep -A20 "isInterpretedName"
# Check beautifyName more carefully - does it expect a specific type?
echo ""
echo "=== beautifyName signature and usage ==="
sed -n '65,85p' packages/ensnode-sdk/src/ens/names.tsRepository: namehash/ensnode Length of output: 546 🏁 Script executed: #!/bin/bash
# Verify the exact flow: does NamedIdentity.name (which is Name) ever get a non-interpreted name?
echo "=== NamedIdentity interface full definition ==="
sed -n '70,105p' packages/ensnode-sdk/src/identity/types.ts
# Check if there's any validation/interpretation before NameDisplay gets the name
echo ""
echo "=== Check DisplayIdentity to see how identity.name flows to NameDisplay ==="
sed -n '160,200p' packages/namehash-ui/src/components/identity/ResolveAndDisplayIdentity.tsx
# One more check: does a NormalizedName always pass isInterpretedName?
echo ""
echo "=== Test with actual examples ==="
cat > /tmp/test_interpretation.ts << 'EOF'
// A NormalizedName is a string that is normalized
// isInterpretedLabel checks: if starts with '[' -> is it valid encoded labelhash? else -> isNormalizedLabel?
// So a NormalizedName where all labels are normalized would pass isInterpretedName
// Example 1: "vitalik.eth" (NormalizedName)
// - Labels: "vitalik" (normalized), "eth" (normalized)
// - isInterpretedLabel("vitalik") = isNormalizedLabel("vitalik") = true
// - isInterpretedLabel("eth") = isNormalizedLabel("eth") = true
// - isInterpretedName returns true ✓
// Example 2: "name" where some labels aren't normalized and aren't encoded hashes (Name but not InterpretedName)
// - e.g., "ViTaLiK.eth" (uppercase, not normalized)
// - Labels: "ViTaLiK" (NOT normalized), "eth"
// - isInterpretedLabel("ViTaLiK") = isNormalizedLabel("ViTaLiK") = false (not normalized, doesn't start with '[')
// - isInterpretedName returns false ✗
// So the guard WILL reject valid displayable names that aren't normalized or encoded
EOF
cat /tmp/test_interpretation.tsRepository: namehash/ensnode Length of output: 3100 Over-strict guard hides valid display input and changes Line 18 blocks 💡 Minimal fix to preserve input while still avoiding strict beautification on invalid names export function NameDisplay({ name, className = "nhui:font-medium" }: NameDisplayProps) {
if (!isInterpretedName(name)) {
- return <span className={className}>(invalid name)</span>;
+ return <span className={className}>{name}</span>;
}
const beautifiedName = beautifyName(name);
return <span className={className}>{beautifiedName}</span>;
}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||
|
|
||||||||||||||||||||
| const beautifiedName = beautifyInterpretedName(name); | ||||||||||||||||||||
| return <span className={className}>{beautifiedName}</span>; | ||||||||||||||||||||
| } | ||||||||||||||||||||

There was a problem hiding this comment.
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.