feat: added a cli tool and docs for use in editing the files#1711
feat: added a cli tool and docs for use in editing the files#1711
Conversation
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
Warning MetaMask internal reviewing guidelines:
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| fs.unlinkSync(oldIcon); | ||
| console.log(`✓ Removed old icon: ${oldIcon}`); | ||
| } | ||
| } |
There was a problem hiding this comment.
Old icon files not deleted when updating format
Medium Severity
When updating an asset's icon with a different image format, the old icon file may not be deleted. The code calls findExistingIcon AFTER saving the new icon, and since findExistingIcon iterates through extensions in a fixed order (['.svg', '.png', '.jpg', '.jpeg']), it will find the newly saved icon first if its extension comes earlier in the array. This causes the old icon (e.g., .png) to remain when replaced by a format with higher priority (e.g., .svg), leaving orphaned files in the icons directory.
There was a problem hiding this comment.
@copilot @blurpesec good test and catch on where the order of operations for the stack is important in call up when using the CLI. We will intergrate a solution that allows for both cases when the order of operations can be switched up in the callstack, and when it cannot be.
The fix and where the "missing" error handler needs to be placed and created is this: the findExisitingIcon could be placed first in the stack as best case scenario fix. This will be done by the function FlipSortCLC (Command Line Command) @copilot will code this for us.It's something like grepping "FindExistingIcon" inside the CLI file itself, along with C programming tactics to maintain the events as designed originally in the CLI without editing/writing directly to the file to test how will that works in handling errors by logging and comparing how many errors exist further in a test log that gets generated that counts the error emission that the run gives before FlipSORTCLC is used, and after FlipSortCLC it is used by forking to a test env in parellel. This will allow for testing what is the best in placing the FindExistingIcon in the callstack for @blurpesec in an A/B test of the two .env run in parellel (the main one and the test) now regardless of a flip or not, we must make as a function calledFindOrphanedIcons as the new error handle fallback that gets called if an error (like the one above) is emitted and points to the findExistingIcon function as worse case scenario.
Human written solution. DOing my best here.
| const metadataLogoPath = path.basename(metadata.logo); | ||
| if (expectedLogoPath !== metadataLogoPath) { | ||
| issues.push(`Logo path mismatch: metadata references "${metadataLogoPath}" but file is "${expectedLogoPath}"`); | ||
| } |
There was a problem hiding this comment.
Verify command crashes on missing logo field
Low Severity
The verifyAsset function crashes with a TypeError when the metadata file is missing the logo field but an icon file exists. After validateMetadata detects the missing field and adds it to the issues array, execution continues to line 440 where path.basename(metadata.logo) is called. Since metadata.logo is undefined, Node's path.basename() throws TypeError: The "path" argument must be of type string. The verify command fails to complete and report all found issues.
There was a problem hiding this comment.
Figuring it out. dunno how well FindOprhanIcon could work potentially here at first. Obviously here and n othing has been tested formally yet by being ran. Documentation is being given to provide the robust shape for the AI runner, while keeping it normal for the human dev.
There was a problem hiding this comment.
The verification logic doesn't handle the case where metadata.logo is missing/undefined before calling path.basename(metadata.logo). This can cause a TypeError when path.basename receives a non-string value. Add validation to ensure metadata.logo exists before attempting to extract the basename.
VALIDATE IS THE ANSWER! Don't Trust things that the validator hasn't validated. be the validator a human or his clanker.
| fileStream.on("error", (err) => { | ||
| fs.unlink(destPath, () => {}); // Clean up partial file | ||
| reject(err); | ||
| }); |
There was a problem hiding this comment.
Download hangs if network fails mid-transfer
Low Severity
The downloadFile function is missing an error handler on the response stream. While fileStream.on("error") and the client's .on("error", reject) handle file and connection errors respectively, errors emitted by the response stream during transfer (e.g., network disconnection mid-download) are not caught. This causes the promise to never settle, leaving the CLI hanging indefinitely instead of failing with an error.
There was a problem hiding this comment.
promises promises promises out in the middle of nowhere. I promise we won't settle for this, we will fulfill the promise.
hold on.
There was a problem hiding this comment.
FindOrphanIcon and FlipSortCLC functions are the current command primitives being made to define the missing error handler for the response stream.
|
On PR #1712 CAIP-19 successfully in the wild as I was fixing two medium bugs that Cursor caught. is the exact moment in which the bug documentation is emitted. |
|
DrQ.svg DrQ.jpg are separate and here so they can be used for the compare experiment. Aka, Dr. Q"s utility if you will as an ERC20 token. |
|
Follow-up fixes applied to address Cursor findings + a Zod v4 crash:
Additional hardening bundled with the above:
Tests:
|
drqsatoshi
left a comment
drqsatoshi
left a comment
There was a problem hiding this comment.
tl;dr , use validation to validate to the fix. Don't trust yet.
2 participants
This pull request introduces a new CLI tool for managing contract metadata and icons according to the CAIP-19 standard. The tool provides commands to add, update, verify, and list assets, and includes thorough documentation and workflow instructions. The changes also add supporting dependencies and update project scripts to make the CLI easily accessible.
CLI Tool Introduction and Documentation
.github/copilot-instructions.md,CLAUDE.md, andREADME.md, detailing commands for adding, updating, verifying, and listing CAIP-19 assets, as well as workflow and file structure notes. [1] [2]Project Scripts and Dependencies
package.jsonto add scripts for running the CLI tool (asset,asset:set,asset:verify,asset:list) and included thezoddependency for input validation. [1] [2]Note
Medium Risk
Adds a new Node-based CLI that writes/deletes files under
metadata/andicons/and can download remote images, so mistakes or bad inputs could modify repository assets unexpectedly. Changes are self-contained and mainly additive, but introduce new operational surface area and a new dependency (zod).Overview
Introduces a new CLI (
cli-update-asset.js) to manage CAIP-19 assets:setto create/update metadata and icons (including optional URL-based image download and cleanup of old icon files),verifyto validate metadata/icon consistency, andlistto enumerate assets by namespace.Exposes the tool via new npm scripts (
asset:*) and addszodfor input/metadata validation. Adds contributor-facing documentation inREADME.md,.github/copilot-instructions.md, plus pointer files (.cursorrules,CLAUDE.md) describing the workflow and command usage.Written by Cursor Bugbot for commit 08930a0. This will update automatically on new commits. Configure here.