1 Commits

Author	SHA1	Message	Date
Hyeonsang Cho	9b0c7cbc37	fix(browser): never replace an existing output file with browser_screenshot (#6913) ... * fix(browser): never replace an existing output file with browser_screenshot browser_screenshot published straight onto outputs/<name> through the replacing atomic publisher, so a screenshot saved as report.png silently replaced the user's own report.png, an earlier capture, or another default-named capture taken in the same second (the default stem has second resolution). The sibling web_capture tool already keeps existing files and takes a -1, -2, ... suffix. Stage the complete capture under a hidden sibling and hard-link it onto the first free candidate name. The link is the whole no-overwrite guard: it fails with FileExistsError when the name exists as anything, so no name is checked before it is claimed and a concurrent writer cannot be replaced; the final name never shows a partial file. Staging shares the outputs directory, so the link is same-filesystem, and like Gateway uploads other link failures propagate instead of falling back to replacement. New files keep the publisher's normal creation mode. Hidden per-action progress frames keep the replacing _write_screenshot: their names carry microsecond timestamps in a server-owned directory. * docs(changelog): reference #6913 for the browser_screenshot no-overwrite fix * fix(browser): refuse a staged screenshot swapped before it is linked The sandbox can write outputs/, so it can see the hidden staged capture and replace it with a symlink before the hard link. os.link follows symlinks by default, which would publish a hard link to the host file as the screenshot artifact (the replacing publisher this PR moved away from never followed one). Stage the capture with an exclusively created descriptor and keep its identity from fstat, link without following symlinks where the platform supports it (decided once at import), and require the published name to be that inode; anything else is removed and the capture fails, like Gateway uploads' identity check. A failed staging write still leaves no files behind. * docs(readme): note that browser_screenshot never replaces an output file * fix(browser): keep the staged screenshot open until its link is verified Linux filesystems reuse a freed inode number at once. Closing the staged capture before linking let a file swapped in at the staged name take the same number and pass the samestat check, so the symlink and planted-file swap tests failed on Linux CI while passing on macOS. Hold the staged descriptor through the link and identity check, as Gateway uploads do, and require the published name to be a regular file. * test(browser): race the peer claim against the real publication step The claimed-name test patched _publish_bytes_atomically, which the screenshot path no longer calls, so its hook never fired and the race assertions passed vacuously. Plant the peer file right before os.link or os.replace lands bytes on a visible name, and assert that the claim happened, the capture moved to report-1.png, and the peer file survived. * refactor(capture): share one output-name candidate sequence browser_screenshot hand-copied web_capture's probe bound and suffix order, and the copies had already drifted: Browserless retried a held timestamped name five times, the screenshot tool once. Move the bound, the retry count and the candidate order into utils/output_names.py and have both tools iterate it, so the naming cannot drift again. Browserless behavior is unchanged; its tests now patch the shared module. --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com>	2026-10-18 10:52:22 +08:00