fix(inspector): say the folder is already opening on a busy Show in folder

A 429 busy from the reveal route now toasts "The folder is already
opening." The browser workflow checks that the reveal POST carries the
page's own Origin and an application/json body, and TESTING.md D38 records
the tightened route, the toast table and the accepted residual risk of a
header-less local proxy.
This commit is contained in:
Vito Sansevero
2026-10-20 21:39:55 -07:00
parent 6c4c587d9e
commit ff59d9b086
5 changed files with 65 additions and 25 deletions
+47 -18
View File
@@ -607,25 +607,54 @@ Recorded deviations on this page (spec §8):
viewer's File & provenance the Path value stays text, followed in its `dd` by a
"Show in folder" button (`Show <filename> in folder`), only for a listed file
(`root_index`, `root_scope`, `path`), so never in the side panel or a recorded run.
It POSTs that identity to `/prompt_manager/images/reveal` and toasts "Opened the
folder on this computer.", "Show in folder works only in a browser on the computer
running ComfyUI." (403 `not-local`), "No file manager is available on this
computer." (501 `unavailable`) or "The folder could not be opened. Try again."
The route is local-only: a loopback `request.remote`, none of `Forwarded`,
`X-Forwarded-For`, `X-Real-IP` or `X-Forwarded-Host`, and (against DNS rebinding) a
loopback `Host` and, when sent, `Origin` (`localhost`, `*.localhost` or a loopback
address). The body is strict (an int root, a 64-hex scope, a bounded relative path;
absolute, drive and `..` paths are 400), the scope goes through
It POSTs that identity as JSON to `/prompt_manager/images/reveal` and toasts:
| Result | Toast |
|---|---|
| Success | "Opened the folder on this computer." |
| 403 `not-local` | "Show in folder works only in a browser on the computer running ComfyUI." |
| 501 `unavailable` | "No file manager is available on this computer." |
| 429 `busy` | "The folder is already opening." |
| Anything else (415, 404, 409, 500, offline) | "The folder could not be opened. Try again." |
The route is local-only. It answers 403 `not-local` unless all of these hold: no
forwarding-style header (`Forwarded`, `Via`, `X-Real-IP`, `CF-Connecting-IP`,
`True-Client-IP`, `X-Client-IP`, `X-Cluster-Client-IP`, `Fastly-Client-IP`, and any
`X-Forwarded-*`, `X-Original-*` or `*-Client-IP`); a loopback `request.remote`; a
loopback `Host` (`localhost`, `*.localhost` or a loopback address, against DNS
rebinding); an `Origin` equal to the request's own scheme, host and port (absent,
`null`, another port or another scheme is refused; browsers send `Origin` on every
fetch POST); and, when sent, `Sec-Fetch-Site: same-origin`. A body that is not
`application/json` is 415 `unsupported-media-type`, so any cross-origin request
needs a CORS preflight. The body is strict (an int root, a 64-hex scope, a bounded
relative path; absolute, drive and `..` paths are 400), the scope goes through
`_gallery_scope_error` (409 `source-changed`), and the path resolves only inside
that root to a regular media file (403/404). `utils/file_reveal.py` starts the
opener as an argv list with an absolute executable, no shell, DEVNULL streams and
`close_fds`, never waited on: Windows `explorer.exe` from `%SystemRoot%` (never
PATH or the working directory) with `/select,` and the path, macOS `open -R`,
Linux the FileManager1 `ShowItems` dbus call (waited at most 2 s, only to detect a
failed start) and else `xdg-open` on the folder. `tests/test_image_reveal.py` mocks
`Popen`, `sys.platform` and `shutil.which`; the browser server's opener is a stub
(`POST`/`GET /__test__/reveal`). The inspector comparison reads closed disclosures
as hidden, so the button needs no mapping there.
that root to a regular media file (403/404; a resolution `OSError` such as
`ENAMETOOLONG` is the same JSON 404, and the client's path is never logged at error
level). One reveal runs at a time per process with a 1 s cooldown after it
(`RevealGate`, 429 `busy`), on its own single worker (`reveal_executor`), never
ComfyUI's default executor. Just before launching, `utils/file_reveal.py` re-checks
that the real path is still a regular file inside the root (a swapped parent is
404). Windows starts no process: `ILCreateFromPathW` and
`SHOpenFolderAndSelectItems` select the file through the shell API (Explorer's own
command line splits on commas, so a path like `output\a,\Windows\...` must never
reach it), falling back to `os.startfile(folder, "explore")`. macOS runs
`/usr/bin/open -R`; Linux asks FileManager1 `ShowItems` with `dbus-send
--print-reply --reply-timeout=2000` (a real reply or a fallback) and else runs
`xdg-open` on the folder. Those openers are argv lists resolved only on a fixed
system PATH, with no shell, DEVNULL streams, `close_fds`, their own session, a
minimal environment (no venv PATH, PYTHONPATH or API keys) and a daemon-thread
reaper. `tests/test_image_reveal.py` mocks `Popen`, `sys.platform`, `shutil.which`,
the shell API and `os.startfile`; the browser server's opener is a stub
(`POST`/`GET /__test__/reveal`) with no cooldown. The inspector comparison reads
closed disclosures as hidden, so the button needs no mapping there.
Residual risk, accepted: a reverse proxy or tunnel that adds no forwarding header
(a bare nginx `proxy_pass` to 127.0.0.1, `ssh -R`, `socat`) cannot be told apart
from a local client, and a non-browser client behind it can forge a matching
`Host` and `Origin`. Such a client can open a folder of a listed file on the host's
screen, at most once a second, and learn whether that file exists (which the
listing routes already tell it). Operators who expose ComfyUI that way accept this.
- **D39**: a recipe-linked image shows its recipe's own category and tags (#224), not
its positive's. The concept shows the prompt's category and tags for every image;
the concept fixture's recipes have neither, so their images show no category chip
+7
View File
@@ -283,6 +283,9 @@ test('Show in folder asks the server to open the listed file and announces each
const toast=page.locator('#workspace-toast');
const [sent]=await Promise.all([page.waitForRequest(entry=>entry.url().endsWith('/prompt_manager/images/reveal')),show.click()]);
expect(sent.method()).toBe('POST');
// The route takes only a same-origin JSON request, as a real browser sends it.
const sentHeaders=await sent.allHeaders();
expect(sentHeaders.origin).toBe(new URL(page.url()).origin);expect(sentHeaders['content-type']).toBe('application/json');
const body=sent.postDataJSON();
expect(Object.keys(body).sort()).toEqual(['path','root_index','root_scope']);
expect(body).toMatchObject({root_index:0,path:'inspector/complete.png'});expect(body.root_scope).toMatch(/^[0-9a-f]{64}$/);
@@ -293,6 +296,10 @@ test('Show in folder asks the server to open the listed file and announces each
await expect(await request.post('/__test__/reveal',{data:{outcome}})).toBeOK();
await show.click();await expect(toast).toHaveText(message);
}
// A reveal already in progress answers 429 busy.
await page.route('**/prompt_manager/images/reveal',route=>route.fulfill({status:429,contentType:'application/json',body:JSON.stringify({success:false,code:'busy',error:'The folder is already opening.'})}));
await show.click();await expect(toast).toHaveText('The folder is already opening.');
await page.unroute('**/prompt_manager/images/reveal');
// A proxied request counts as remote, so the real route answers 403 not-local.
await expect(await request.post('/__test__/reveal',{data:{outcome:'opened'}})).toBeOK();
await page.route('**/prompt_manager/images/reveal',route=>route.continue({headers:{...route.request().headers(),'x-forwarded-for':'203.0.113.7'}}));
+2 -1
View File
@@ -82,6 +82,7 @@ test('Show in folder outcomes use the D38 toasts',async()=>{
assert.equal(revealOutcome(null),'Opened the folder on this computer.');
assert.equal(revealOutcome({status:403,code:'not-local'}),'Show in folder works only in a browser on the computer running ComfyUI.');
assert.equal(revealOutcome({status:501,code:'unavailable'}),'No file manager is available on this computer.');
for(const error of [{status:403,code:null},{status:409,code:'source-changed'},{status:500},new TypeError('offline')])
assert.equal(revealOutcome({status:429,code:'busy'}),'The folder is already opening.');
for(const error of [{status:403,code:null},{status:409,code:'source-changed'},{status:415,code:'unsupported-media-type'},{status:429,code:null},{status:500},new TypeError('offline')])
assert.equal(revealOutcome(error),'The folder could not be opened. Try again.');
});
+7 -6
View File
@@ -1,11 +1,12 @@
"""Show one gallery file in this computer's file manager (TESTING.md D38).
The route that calls this is local-only (``is_local_request``), single-flight
(``RevealGate``) and hands in a
path it has already resolved inside a gallery root. Windows goes through the
shell API (no command line exists to parse); macOS and Linux openers run as an
argument list with an absolute executable, no shell and no inherited streams,
so neither a file name nor ``PATH`` can change what runs or what it parses.
The route that calls this is local-only (``is_local_request``) and
single-flight (``RevealGate``), and hands in a path it has already resolved
inside a gallery root, which is re-checked just before launching. Windows
goes through the shell API (no command line exists to parse); macOS and Linux
openers run as an argument list with an absolute executable from a fixed
system PATH, no shell, no inherited streams and a minimal environment, so
neither a file name nor ``PATH`` can change what runs or what it parses.
"""
import ipaddress
+2
View File
@@ -22,6 +22,7 @@ const REVEAL_URL = '/prompt_manager/images/reveal';
const REVEAL_OPENED = 'Opened the folder on this computer.';
const REVEAL_NOT_LOCAL = 'Show in folder works only in a browser on the computer running ComfyUI.';
const REVEAL_UNAVAILABLE = 'No file manager is available on this computer.';
const REVEAL_BUSY = 'The folder is already opening.';
const REVEAL_FAILED = 'The folder could not be opened. Try again.';
const FACTS = [['seed', 'Seed'], ['steps', 'Steps'], ['cfg', 'CFG'], ['sampler', 'Sampler'], ['scheduler', 'Scheduler'], ['model', 'Model']];
@@ -83,6 +84,7 @@ export function revealOutcome(error) {
if (error === null) return REVEAL_OPENED;
if (error?.status === 403 && error.code === 'not-local') return REVEAL_NOT_LOCAL;
if (error?.status === 501) return REVEAL_UNAVAILABLE;
if (error?.status === 429 && error.code === 'busy') return REVEAL_BUSY;
return REVEAL_FAILED;
}