Add ComfyUI node compatibility checker#973
Conversation
Scan workflow templates against a local ComfyUI object registry to flag stale node inputs, deprecated nodes, and removed API model options before templates break at runtime.
📝 WalkthroughWalkthroughAdds a ComfyUI node compatibility validator with shared path helpers, live and static node-spec builders, workflow template scanning, report generation, a new npm command, and a GitHub Actions workflow that posts PR comments and uploads artifacts. ChangesComfyUI node compatibility validation
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/validate/check_comfyui_node_compat.py`:
- Line 33: The hardcoded personal default in DEFAULT_COMFYUI_DIR should be
removed because it breaks other users and leaks a username. Update
check_comfyui_node_compat.py to derive paths through scripts/lib/paths.py or
require an explicit source instead of defaulting to
/Users/linmoumou/Documents/Github/ComfyUI; then make main() validate that one of
--comfyui-dir, --object-info-url, or --object-info-json is provided before any
filesystem access.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: c59da490-9dc7-4e8d-8041-aad64879bb52
📒 Files selected for processing (2)
package.jsonscripts/validate/check_comfyui_node_compat.py
…porting. Move the checker to scripts/comfyui_node_compat/, add static-scan CI workflow that posts informational PR comments, prioritize log output by severity, and document usage in README/AGENTS/scripts README.
ComfyUI Node Compatibility ReportStatus: no report generated. Informational only — this check does not block merging. See workflow logs for details. This comment is automatically updated by the ComfyUI Node Compatibility Report workflow. |
Remove script and workflow path triggers so the informational scan and PR comment are posted only when template assets are modified.
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/README.md:
- Around line 75-84: The markdown in this README section violates MD031 because
the fenced code blocks are not separated by blank lines. Update the surrounding
content near the local testing examples so each fence in the README has an empty
line before and after it, keeping the text and the two code blocks clearly
separated.
In @.github/workflows/report-comfyui-node-compat.yml:
- Around line 25-37: The workflow checkout in the report job should be hardened
by disabling persisted GitHub credentials and, if possible, limiting the
pull-requests permission to only the report job instead of the entire workflow.
Update the actions/checkout step in the workflow’s checkout setup to set
persist-credentials to false, and adjust the workflow-level permissions so the
job that posts the report owns pull-requests: write. Also verify whether the
repo policy requires pinning actions in this workflow before making any broader
changes.
- Around line 112-118: The comment lookup in the GitHub Action only checks the
first page of results, so `existing` can miss the `<!--
comfyui-node-compat-report -->` marker on busy PRs and create duplicates. Update
the `listComments` call to use pagination (for example via `github.paginate` or
repeated page fetching) before running the `comments.find(...)` check. Keep the
existing marker-based match logic in the workflow step so the update path still
finds the prior report comment.
- Around line 6-11: The workflow trigger currently allows overlapping runs on
the same pull request, which can cause duplicate or stale comment updates. Add a
per-PR concurrency configuration to the report-comfyui-node-compat workflow so
only the latest run for a given PR stays active and earlier ones are canceled.
Use the workflow’s existing pull_request context with a stable group key and
enable cancel-in-progress to keep runs orderly.
- Around line 6-15: Forked pull requests cannot safely use a write-enabled
GITHUB_TOKEN with the current pull_request-based commenter workflow, so update
the comment-posting flow in the report-comfyui-node-compat workflow to avoid
token downgrade failures. Refactor the workflow trigger/setup to use a safer
pattern for external contributions, such as a workflow_run-driven comment step,
and ensure the job that posts comments no longer relies on pull_request
permissions that will be read-only for fork PRs.
In `@package.json`:
- Line 19: The new package script uses a different Python interpreter than the
sibling validation scripts, so align `validate:comfyui-nodes` with the existing
`python`-based entries in package.json. Update the script in the package.json
scripts section to use the same binary as the other validation commands so all
checks invoke a consistent interpreter.
In `@README.md`:
- Around line 586-591: The Markdown section in README.md needs a blank line
before the fenced code block to satisfy MD031. Update the “Run locally before
committing:” section so the fence in the README sits after an empty line,
keeping the existing commands and surrounding prose intact.
In `@scripts/comfyui_node_compat/registry.py`:
- Around line 73-78: The current probe in probe_object_info_url() is doing a
full load via load_object_info_from_url(), which duplicates the downstream
fetch/parse work and wastes a round-trip. Update probe_object_info_url() to
perform only a lightweight reachability check (or remove the probe entirely and
let the caller load once and branch on success/failure), and make sure the
check.py caller path uses a single request/parse flow instead of calling
load_object_info_from_url() twice.
- Around line 65-70: load_object_info_from_url currently trusts any URL accepted
by urllib.request.urlopen, which can allow local-file or other non-web schemes
to be fetched; update this function to validate the parsed URL scheme before
opening it, and only allow http and https. Keep the existing JSON parsing and
dict validation, but reject anything else early with a clear error so the fetch
stays remote-only.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 4cb66885-0b72-4ddc-9e52-f388b3c08f48
📒 Files selected for processing (17)
.env.example.github/workflows/README.md.github/workflows/report-comfyui-node-compat.yml.gitignoreAGENTS.mdREADME.mdpackage.jsonscripts/README.mdscripts/comfyui_node_compat/__init__.pyscripts/comfyui_node_compat/check.pyscripts/comfyui_node_compat/clone.pyscripts/comfyui_node_compat/models.pyscripts/comfyui_node_compat/registry.pyscripts/comfyui_node_compat/report.pyscripts/comfyui_node_compat/static_registry.pyscripts/comfyui_node_compat/workflow.pyscripts/lib/paths.py
| **Local testing (full coverage — needs running ComfyUI):** | ||
| ```bash | ||
| npm run validate:comfyui-nodes | ||
| # Reports: comfyui-node-compat.latest.log, comfyui-node-compat.log (gitignored) | ||
| ``` | ||
|
|
||
| **Local testing (CI-style static scan):** | ||
| ```bash | ||
| python3 scripts/comfyui_node_compat/check.py --static-scan --clone-comfyui --no-fail | ||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Surround fenced code blocks with blank lines (MD031).
markdownlint flags the fences at Lines 76 and 82 — a blank line above each keeps the renderer (and the linter) from fencing with you.
📝 Add blank lines around fences
**Local testing (full coverage — needs running ComfyUI):**
+
```bash
npm run validate:comfyui-nodes
# Reports: comfyui-node-compat.latest.log, comfyui-node-compat.log (gitignored)Local testing (CI-style static scan):
+
python3 scripts/comfyui_node_compat/check.py --static-scan --clone-comfyui --no-fail</details>
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **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.
```suggestion
**Local testing (full coverage — needs running ComfyUI):**
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 76-76: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 82-82: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/README.md around lines 75 - 84, The markdown in this
README section violates MD031 because the fenced code blocks are not separated
by blank lines. Update the surrounding content near the local testing examples
so each fence in the README has an empty line before and after it, keeping the
text and the two code blocks clearly separated.
Source: Linters/SAST tools
| on: | ||
| pull_request: | ||
| types: [opened, synchronize] | ||
| paths: | ||
| - 'templates/**' | ||
| workflow_dispatch: |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Add a concurrency group to cancel superseded runs.
Rapid synchronize pushes can launch overlapping runs that race on the same PR comment (duplicate posts, stale content). A per-PR concurrency group keeps things orderly — first come, last served.
🔒 Suggested concurrency block
on:
pull_request:
types: [opened, synchronize]
paths:
- 'templates/**'
workflow_dispatch:
+
+concurrency:
+ group: comfyui-node-compat-${{ github.event.pull_request.number || github.ref }}
+ cancel-in-progress: true📝 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.
| on: | |
| pull_request: | |
| types: [opened, synchronize] | |
| paths: | |
| - 'templates/**' | |
| workflow_dispatch: | |
| on: | |
| pull_request: | |
| types: [opened, synchronize] | |
| paths: | |
| - 'templates/**' | |
| workflow_dispatch: | |
| concurrency: | |
| group: comfyui-node-compat-${{ github.event.pull_request.number || github.ref }} | |
| cancel-in-progress: true |
🧰 Tools
🪛 zizmor (1.26.1)
[warning] 6-11: insufficient job-level concurrency limits (concurrency-limits): workflow is missing concurrency setting
(concurrency-limits)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/report-comfyui-node-compat.yml around lines 6 - 11, The
workflow trigger currently allows overlapping runs on the same pull request,
which can cause duplicate or stale comment updates. Add a per-PR concurrency
configuration to the report-comfyui-node-compat workflow so only the latest run
for a given PR stays active and earlier ones are canceled. Use the workflow’s
existing pull_request context with a stable group key and enable
cancel-in-progress to keep runs orderly.
Source: Linters/SAST tools
| on: | ||
| pull_request: | ||
| types: [opened, synchronize] | ||
| paths: | ||
| - 'templates/**' | ||
| workflow_dispatch: | ||
|
|
||
| permissions: | ||
| contents: read | ||
| pull-requests: write |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major
🧩 Analysis chain
🌐 Web query:
Does GitHub downgrade GITHUB_TOKEN to read-only for pull_request workflows triggered from forks even when pull-requests write permission is set?
💡 Result:
Yes, GitHub intentionally downgrades the GITHUB_TOKEN to read-only for workflows triggered by pull request events from forked repositories [1][2][3]. Even if you explicitly define write permissions in your workflow configuration (using the permissions key) or set repository-level default permissions to read/write, GitHub enforces this read-only restriction for security reasons to prevent malicious code in a fork from potentially compromising the base repository [1][3][4][5]. There is a specific repository-level setting called "Send write tokens to workflows from pull requests" that, when enabled, allows pull requests from forks to receive a GITHUB_TOKEN with write permissions [6][7]. However, this is disabled by default for security and must be explicitly enabled by a repository administrator [6][7]. For workflows that require write access (e.g., to post comments on a PR), a common architectural pattern is to use a two-stage workflow [4]: 1. A first, untrusted workflow (triggered by pull_request) runs on the fork's code with read-only permissions and saves necessary information as workflow artifacts [4]. 2. A second, trusted workflow (triggered by workflow_run) runs on the base repository's context, has access to write permissions, and consumes the artifacts produced by the first workflow to perform the desired actions (like posting a comment) [4]. Alternatively, the pull_request_target event can be used to run workflows in the context of the base repository, which does not receive this automatic read-only downgrade, but this approach carries significant security risks if not handled carefully, as it executes code from the pull request using the base repository's permissions [3][8][5].
Citations:
- 1: https://github.blog/changelog/2021-04-20-github-actions-control-permissions-for-github_token/
- 2: https://docs.github.com/en/enterprise-server@3.0/actions/reference/authentication-in-a-workflow
- 3: Clarify Permissions Calculation for Workflow Jobs in Documentation github/docs#33566
- 4: CI: PR comments fail for PRs from forked repositories tenstorrent/tt-inference-server#1671
- 5: https://stackoverflow.com/questions/78444367/github-token-permission-in-workflow-changes-to-read-after-setting-it-to-write
- 6: https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/enabling-features-for-your-repository/managing-github-actions-settings-for-a-repository
- 7: https://docs.github.com/en/organizations/managing-organization-settings/disabling-or-limiting-github-actions-for-your-organization
- 8: Clarify
How the permissions are calculated for a workflow jobgithub/docs#32398
Fork PRs can't post comments with a pull_request trigger—don't let the token go rogue!
For pull requests originating from forks, GitHub strictly downgrades the GITHUB_TOKEN to read-only, even if you explicitly grant pull-requests: write in your workflow. The repository's guardian goblins enforce this to prevent malicious code from slipping through the cracks. Attempting to comment will cause your job to stumble and turn red for external contributors.
Since this repo welcomes fork contributions, you should verify the intended behavior. The usual safe spell is a workflow_run-based commenter pattern to avoid token traps and keep the deed done without the dread.
🧰 Tools
🪛 zizmor (1.26.1)
[error] 15-15: overly broad permissions (excessive-permissions): pull-requests: write is overly broad at the workflow level
(excessive-permissions)
[warning] 15-15: permissions without explanatory comments (undocumented-permissions): needs an explanatory comment
(undocumented-permissions)
[warning] 6-11: insufficient job-level concurrency limits (concurrency-limits): workflow is missing concurrency setting
(concurrency-limits)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/report-comfyui-node-compat.yml around lines 6 - 15, Forked
pull requests cannot safely use a write-enabled GITHUB_TOKEN with the current
pull_request-based commenter workflow, so update the comment-posting flow in the
report-comfyui-node-compat workflow to avoid token downgrade failures. Refactor
the workflow trigger/setup to use a safer pattern for external contributions,
such as a workflow_run-driven comment step, and ensure the job that posts
comments no longer relies on pull_request permissions that will be read-only for
fork PRs.
| - name: Checkout workflow templates (JSON only) | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| ref: ${{ github.event.pull_request.head.sha || github.sha }} | ||
| sparse-checkout: | | ||
| templates/*.json | ||
| scripts/comfyui_node_compat/ | ||
| scripts/lib/paths.py | ||
| scripts/lib/locale_index_files.py | ||
| sparse-checkout-cone-mode: false | ||
|
|
||
| - name: Set up Python | ||
| uses: actions/setup-python@v5 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔵 Trivial | 💤 Low value
Optional hardening per static analysis (zizmor).
A few low-risk posture nits worth a once-over: set persist-credentials: false on the checkout (credentials aren't needed downstream), and consider scoping pull-requests: write to the report job rather than the whole workflow. Pinning actions to commit SHAs is also flagged — confirm whether this repo's other workflows follow that blanket policy before churning here.
🛡️ persist-credentials
- name: Checkout workflow templates (JSON only)
uses: actions/checkout@v4
with:
ref: ${{ github.event.pull_request.head.sha || github.sha }}
+ persist-credentials: false
sparse-checkout: |📝 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.
| - name: Checkout workflow templates (JSON only) | |
| uses: actions/checkout@v4 | |
| with: | |
| ref: ${{ github.event.pull_request.head.sha || github.sha }} | |
| sparse-checkout: | | |
| templates/*.json | |
| scripts/comfyui_node_compat/ | |
| scripts/lib/paths.py | |
| scripts/lib/locale_index_files.py | |
| sparse-checkout-cone-mode: false | |
| - name: Set up Python | |
| uses: actions/setup-python@v5 | |
| - name: Checkout workflow templates (JSON only) | |
| uses: actions/checkout@v4 | |
| with: | |
| ref: ${{ github.event.pull_request.head.sha || github.sha }} | |
| persist-credentials: false | |
| sparse-checkout: | | |
| templates/*.json | |
| scripts/comfyui_node_compat/ | |
| scripts/lib/paths.py | |
| scripts/lib/locale_index_files.py | |
| sparse-checkout-cone-mode: false | |
| - name: Set up Python | |
| uses: actions/setup-python@v5 |
🧰 Tools
🪛 zizmor (1.26.1)
[warning] 25-34: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 26-26: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 37-37: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/report-comfyui-node-compat.yml around lines 25 - 37, The
workflow checkout in the report job should be hardened by disabling persisted
GitHub credentials and, if possible, limiting the pull-requests permission to
only the report job instead of the entire workflow. Update the actions/checkout
step in the workflow’s checkout setup to set persist-credentials to false, and
adjust the workflow-level permissions so the job that posts the report owns
pull-requests: write. Also verify whether the repo policy requires pinning
actions in this workflow before making any broader changes.
Source: Linters/SAST tools
| const { data: comments } = await github.rest.issues.listComments({ | ||
| owner: context.repo.owner, | ||
| repo: context.repo.repo, | ||
| issue_number: context.issue.number, | ||
| }); | ||
|
|
||
| const existing = comments.find((comment) => comment.body.includes(marker)); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Paginate listComments or the single-comment update will multiply on busy PRs.
listComments defaults to 30 results per page, so on a PR with many comments the existing <!-- comfyui-node-compat-report --> marker may sit on a later page and never be found — meaning a fresh comment is created on every run. To keep it a solo act rather than a chorus, paginate.
♻️ Use the paginator
- const { data: comments } = await github.rest.issues.listComments({
- owner: context.repo.owner,
- repo: context.repo.repo,
- issue_number: context.issue.number,
- });
+ const comments = await github.paginate(github.rest.issues.listComments, {
+ owner: context.repo.owner,
+ repo: context.repo.repo,
+ issue_number: context.issue.number,
+ per_page: 100,
+ });📝 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.
| const { data: comments } = await github.rest.issues.listComments({ | |
| owner: context.repo.owner, | |
| repo: context.repo.repo, | |
| issue_number: context.issue.number, | |
| }); | |
| const existing = comments.find((comment) => comment.body.includes(marker)); | |
| const comments = await github.paginate(github.rest.issues.listComments, { | |
| owner: context.repo.owner, | |
| repo: context.repo.repo, | |
| issue_number: context.issue.number, | |
| per_page: 100, | |
| }); | |
| const existing = comments.find((comment) => comment.body.includes(marker)); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/report-comfyui-node-compat.yml around lines 112 - 118, The
comment lookup in the GitHub Action only checks the first page of results, so
`existing` can miss the `<!-- comfyui-node-compat-report -->` marker on busy PRs
and create duplicates. Update the `listComments` call to use pagination (for
example via `github.paginate` or repeated page fetching) before running the
`comments.find(...)` check. Keep the existing marker-based match logic in the
workflow step so the update path still finds the prior report comment.
| "mcp:models": "python scripts/mcp/enhance_models_registry.py", | ||
| "validate:templates": "python scripts/validate/validate_templates.py", | ||
| "validate:manifests": "python scripts/validate/validate_manifests.py", | ||
| "validate:comfyui-nodes": "python3 scripts/comfyui_node_compat/check.py", |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Interpreter inconsistency: python3 here vs python in sibling scripts.
Lines 17-18 invoke python, while this new entry uses python3. On hosts where only one of the two is on PATH, this split could trip a runner up. Pick one binary to keep the scripts in sync and skip the surprise.
♻️ Optional alignment
- "validate:comfyui-nodes": "python3 scripts/comfyui_node_compat/check.py",
+ "validate:comfyui-nodes": "python scripts/comfyui_node_compat/check.py",📝 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.
| "validate:comfyui-nodes": "python3 scripts/comfyui_node_compat/check.py", | |
| "validate:comfyui-nodes": "python scripts/comfyui_node_compat/check.py", |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@package.json` at line 19, The new package script uses a different Python
interpreter than the sibling validation scripts, so align
`validate:comfyui-nodes` with the existing `python`-based entries in
package.json. Update the script in the package.json scripts section to use the
same binary as the other validation commands so all checks invoke a consistent
interpreter.
| Run locally before committing: | ||
| ```bash | ||
| python scripts/sync/sync_bundles.py # Templates | ||
| python scripts/sync/sync_blueprints.py # Blueprints | ||
| npm run validate:comfyui-nodes # Optional: node baseline vs templates (needs running ComfyUI) | ||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add a blank line before the fenced block (MD031).
markdownlint flags the fence at Line 587 — let it breathe with a blank line above.
📝 Blank line before fence
Run locally before committing:
+
```bash
python scripts/sync/sync_bundles.py # Templates📝 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.
| Run locally before committing: | |
| ```bash | |
| python scripts/sync/sync_bundles.py # Templates | |
| python scripts/sync/sync_blueprints.py # Blueprints | |
| npm run validate:comfyui-nodes # Optional: node baseline vs templates (needs running ComfyUI) | |
| ``` | |
| Run locally before committing: | |
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 587-587: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@README.md` around lines 586 - 591, The Markdown section in README.md needs a
blank line before the fenced code block to satisfy MD031. Update the “Run
locally before committing:” section so the fence in the README sits after an
empty line, keeping the existing commands and surrounding prose intact.
Source: Linters/SAST tools
| def load_object_info_from_url(url: str) -> dict[str, Any]: | ||
| with urllib.request.urlopen(url, timeout=30) as response: | ||
| payload = json.loads(response.read().decode("utf-8")) | ||
| if not isinstance(payload, dict): | ||
| raise ValueError("object_info response must be a JSON object") | ||
| return payload |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor
🧩 Analysis chain
🌐 Web query:
Does Python's urllib.request.urlopen support the file:// scheme and read local files?
💡 Result:
Yes, Python's urllib.request.urlopen supports the file:// scheme and can be used to read local files [1][2]. This functionality is handled by the FileHandler class within the urllib.request module [1][2]. When you pass a URL using the file:// scheme to urlopen, it opens the specified local file and returns a file-like object that you can read [1][2]. Important considerations regarding its usage include: - Hostname restriction: By default, FileHandler is designed to handle local files and only supports requests where the hostname is empty or set to 'localhost' [1][2]. Attempting to use a remote hostname with a file:// URL will raise a URLError [1][2]. - Security and behavior: Because it is intended for local access, using this on untrusted input can present security risks similar to other file-system-access operations. - Historical context: While older versions of Python's urllib (prior to Python 3) allowed opening local files without a scheme identifier, modern usage with urllib.request.urlopen generally requires the file:// scheme to be explicitly provided [3]. You can also use the helper functions urllib.request.pathname2url and urllib.request.url2pathname to safely convert between local file paths and file:// URLs [1][2]. Recent updates have further improved the ergonomics of these functions to better handle complete file URIs [4].
Citations:
- 1: https://docs.python.org/3.14/library/urllib.request.html
- 2: https://docs.python.org/3/library/urllib.request.html
- 3: https://stackoverflow.com/questions/20558587/opening-local-file-works-with-urllib-but-not-with-urllib2
- 4: GH-125866: Support complete "file:" URLs in urllib python/cpython#132378
Block local file tricks with a URL scheme sanity check.
The fear is valid: urllib.request.urlopen will happily slurp file:// or ftp:// URLs too, turning a "remote fetch" into a local file eavesdrop if a scheme slips in. Even with operator-controlled input, a simple "file-tyo" shouldn't leak the keys to the kingdom. Restrict the scheme to http/https to keep the data flowing on the right track and avoid a "file" of trouble.
🛡️ Fix in one breath
def load_object_info_from_url(url: str) -> dict[str, Any]:
+ from urllib.parse import urlparse
+
+ if urlparse(url).scheme not in ("http", "https"):
+ raise ValueError("object_info URL must use http or https")
with urllib.request.urlopen(url, timeout=30) as response:
payload = json.loads(response.read().decode("utf-8"))
if not isinstance(payload, dict):📝 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.
| def load_object_info_from_url(url: str) -> dict[str, Any]: | |
| with urllib.request.urlopen(url, timeout=30) as response: | |
| payload = json.loads(response.read().decode("utf-8")) | |
| if not isinstance(payload, dict): | |
| raise ValueError("object_info response must be a JSON object") | |
| return payload | |
| def load_object_info_from_url(url: str) -> dict[str, Any]: | |
| from urllib.parse import urlparse | |
| if urlparse(url).scheme not in ("http", "https"): | |
| raise ValueError("object_info URL must use http or https") | |
| with urllib.request.urlopen(url, timeout=30) as response: | |
| payload = json.loads(response.read().decode("utf-8")) | |
| if not isinstance(payload, dict): | |
| raise ValueError("object_info response must be a JSON object") | |
| return payload |
🧰 Tools
🪛 ast-grep (0.44.0)
[warning] 65-65: Request-controlled URL passed to urlopen; validate against an allowlist to prevent SSRF.
Context: urllib.request.urlopen(url, timeout=30)
Note: [CWE-918] Server-Side Request Forgery (SSRF).
(urlopen-unsanitized-data)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/comfyui_node_compat/registry.py` around lines 65 - 70,
load_object_info_from_url currently trusts any URL accepted by
urllib.request.urlopen, which can allow local-file or other non-web schemes to
be fetched; update this function to validate the parsed URL scheme before
opening it, and only allow http and https. Keep the existing JSON parsing and
dict validation, but reject anything else early with a clear error so the fetch
stays remote-only.
Source: Linters/SAST tools
| def probe_object_info_url(url: str) -> bool: | ||
| try: | ||
| load_object_info_from_url(url) | ||
| return True | ||
| except Exception: | ||
| return False |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🔵 Trivial | ⚖️ Poor tradeoff
probe_object_info_url downloads the entire payload just to throw it away.
This probe calls load_object_info_from_url, which fetches and JSON-parses the full /object_info body — and per the downstream caller (check.py:155-165) the very next step fetches and parses it again. For a large registry that's two round-trips and two parses where one would do. Don't fetch twice and you won't pay twice.
Consider probing reachability cheaply (e.g. a HEAD/lightweight GET) or having the caller load once and branch on success/failure.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/comfyui_node_compat/registry.py` around lines 73 - 78, The current
probe in probe_object_info_url() is doing a full load via
load_object_info_from_url(), which duplicates the downstream fetch/parse work
and wastes a round-trip. Update probe_object_info_url() to perform only a
lightweight reachability check (or remove the probe entirely and let the caller
load once and branch on success/failure), and make sure the check.py caller path
uses a single request/parse flow instead of calling load_object_info_from_url()
twice.
…proved reporting. Add `widget_slot_mismatch` and `invalid_widget_value` issues to README and models. Update workflow and report functions to prioritize critical issues and improve log formatting. Refactor input handling for dynamic combo branches and widget slot alignment checks.
1 participant
Summary
scripts/validate/check_comfyui_node_compat.pyto scan templates against a local ComfyUI node registry or/object_infoendpointnpm run validate:comfyui-nodesas a convenience commandTest plan
python3 scripts/validate/check_comfyui_node_compat.py --help--object-info-url http://127.0.0.1:8188/object_info