Post-registration full re-verify (§14.7) found an unauthenticated CSRF-reachable
arbitrary-directory delete: DatasetProject.make(reset=True) runs shutil.rmtree on an
uncontained os.path.join(output_dir, dataset_name) (nodes/common.py:169-173) — an
absolute or ..-containing name escapes the output dir. Removed from default + node_db/new,
added to node_db/dev with [SECURITY ISSUE] suffix (reversible install-block, §4.7-H mechanic).
Restore to default on author fix (realpath+commonpath containment) + re-verify pass.
PR conflicted on the list tail; manually registered. Deep-verify SAFE (unauth routes are inference/model-load only, no side-effect sink), Non-English PASS.
Relocate ANe5s/ComfyUI-MiniMax-H3-Hybrid (#3154) from array head to tail.
Manually register Flow-Wrangler (#3142), minimax-h3-latent (#3151),
buqi-minimax-h3-multigpu (#3158), H3-Continuum (#3159) — their PRs
conflicted on the list tail after the batch merges. buqi-minimax-h3-multigpu
description trimmed to English-only (author's zh restatement dropped, §4.7-D desc policy).
Sync all 7 into node_db/new (newest-first).
The dialog renders HTML, so run the server body and the pack title
(registry-derived) through the existing sanitizeHTML() at both sites. The
title is wrapped in String() so a missing title can't throw inside the error
handler; rendered text is unchanged for ordinary values.
The batch install/uninstall paths replaced the server's 404 body with a
false "default channel" lead. Drop the 404 branch so a 404 falls through to
the existing server-text path; uninstall never returns 404 anyway. Fixes
#3128 (section 1), reported by plz12345.
write_config() rebuilt config.ini's [default] section entirely from the
startup snapshot into a fresh parser opened 'w'. Any Manager settings
change therefore silently reverted values hand-edited while ComfyUI was
running (the same config['default'] flags those messages tell users to
edit), deleted the two read-but-never-written keys (http_channel_enabled,
default_cache_as_channel_url), and erased every non-[default] section.
get_config() now tracks which keys callers actually change, and
write_config() re-reads config.ini and overlays only those keys plus the
live preview_method. After a successful write the just-persisted keys are
cleared from the tracker, so a key changed once through the UI does not
stay dirty for the life of the process and re-clobber a later hand-edit of
that same key on the next unrelated write. Hand-edited values, unknown
keys, and foreign sections survive; CRLF sanitization still applies to
every value written; an unparsable config.ini falls back to a full rewrite
instead of failing every settings endpoint.
configparser.read() suppresses OSError, so a config.ini that exists but
cannot be read comes back as an empty parser rather than an exception,
which would silently route the merge into the bootstrap branch and rewrite
the file from defaults. write_config() now detects that case (the file
exists but read() loaded nothing) and refuses the write with a raised
error, leaving the file untouched, rather than destroying its contents
while reporting success.
tests/test_write_config_persistence.py pins the three loss classes
(hand-edit revert, key deletion, section erasure), the settings
round-trip, the persisted-key re-clobber guard, and the silent-read-failure
refusal.
The install-denial diagnostics named the wrong gate, sending users to
settings that could not resolve their denial:
- SECURITY_MESSAGE_FLAG_GIT_URL / _FLAG_PIP omitted the loopback half of
the flag-AND-loopback predicate, telling users to enable a flag that
was often already enabled. They now state both conditions, print the
live --listen value at every emission site, and explain that both
values are read once at startup (stop the server, edit, then start).
- SECURITY_MESSAGE_MIDDLE_OR_BELOW instructed setting security_level to
'middle', a value that does not exist; it now names only real values
(any level other than 'strong': normal, normal-, weak).
- The blocked-risk arm reused SECURITY_MESSAGE_GENERAL, blaming
security_level although no value of it can help there; it now emits a
dedicated SECURITY_MESSAGE_BLOCKED_RISK stating this is not a
configuration problem.
- SECURITY_MESSAGE_NORMAL_MINUS_MODEL now names its loopback-listener
condition and the same restart guidance.
- The js/common.js dedicated-403 messages mirror the loopback clause.
Re-add pixaroma/ComfyUI-Pixaroma after verifying the v1.4.82 security
fixes (PR #3118): the four reported filesystem-access issues are now
contained through a shared path guard. Registered to the default list.
Claude-Session: https://claude.ai/code/session_01VKv6FcDs7S642dM1if3vxD
Two defects, the second hidden behind the first. CI runs ruff only, so
neither shows up there.
The generic-403 guard counted the copy across all of js/common.js while its
docstring describes exactly the two occurrences inside handle403Response. A
third, unrelated use at line 722 builds the batch-uninstall error message, so
the file-wide count tripped on it. Scoped the count to the handle403Response
block via the existing _js_function_block helper. This part is not
platform-specific; the count is 3 on Linux and Windows alike.
With that passing, the test reached a loop calling js_file.read_text() with no
encoding. That uses the locale encoding, so on cp1252 Windows it raises
UnicodeDecodeError on the four js files containing emoji. Passed
encoding="utf-8" explicitly, and did the same for the other four read_text
calls in the file, which read ASCII sources today and so pass by luck.
Dariusz Dębicki