Skip to content

fix(r2): reject empty remote manifests before scoped upload - #161

Merged
steipete merged 1 commit into
openclaw:mainfrom
SebTardif:fix/r2-empty-manifest-hit
Sep 5, 2026
Merged

fix(r2): reject empty remote manifests before scoped upload#161
steipete merged 1 commit into
openclaw:mainfrom
SebTardif:fix/r2-empty-manifest-hit

Conversation

@SebTardif

Copy link
Copy Markdown
Contributor

What Problem This Solves

Fixes an issue where a docs publish that received a 200 remote manifest of {}, null, or a JSON array would treat that body as a cache hit. A later locale or page scoped upload then rewrote the remote catalog from that empty set, dropping objects that were not in the current scope.

Why This Change Was Made

getRemoteManifest() already refuses invalid JSON. It did not require entries to be an array before stamping status: "hit". Both the R2 GET path and the local file-path path now require Array.isArray(parsed.entries). Dry-run with no remote file and HTTP 404 stay status: "missing" with empty entries. A valid { version: 1, entries: [], objectCount: 0 } remains a hit.

User Impact

Scoped docs publishes no longer replace the remote R2 catalog with a partial tree when the previous manifest body is empty or the wrong JSON shape. Operators see a fail-closed error instead of a successful upload that deleted unpublished locales.

Evidence

terminal output from running scripts/docs-site/r2-upload.mjs through a small node driver against the unfixed tree, then the patched file.

Before, a file-path remote body of {} during R2_UPLOAD_SCOPE=locale completed as a hit and wrote dist/docs-r2-manifest.locale.merged.json:

$ node r2-empty-manifest-driver.mjs
===== FILE {} scoped locale dry-run =====
status=0
stdout:
r2 remote manifest: hit from file (0 entries)
r2 upload scope: locale (1/1 manifest entries, partial=true)
r2 object cache: 0/1 hits, 1 misses; hit sources: none; miss reasons: missing=1
r2 upload plan: 1/1 changed objects, 0 deleted objects
r2 dry-run put: de/page
r2 dry-run put: .openclaw-docs-r2-manifest.json
r2 upload ok: 1 changed objects, 0 deleted objects plus .openclaw-docs-r2-manifest.json
merged_exists=true

An HTTP 200 body of {} from the synthetic R2 endpoint also completed as a hit:

===== FETCH 200 {} =====
status=0
stdout:
r2 remote manifest: hit from r2 (0 entries)
r2 upload ok: 1 changed objects, 0 deleted objects plus .openclaw-docs-r2-manifest.json

After the patch, the same driver refuses both empty shapes and still accepts a valid empty entries array:

===== FILE {} scoped locale dry-run =====
status=1
stderr:
Error: remote manifest must contain an entries array; refusing to reupload the full docs tree
merged_exists=false

===== FETCH 200 {} =====
status=1
Error: remote manifest must contain an entries array; refusing to reupload the full docs tree

===== FETCH 200 valid empty entries =====
status=0
r2 remote manifest: hit from r2 (0 entries)
r2 upload ok: 1 changed objects, 0 deleted objects plus .openclaw-docs-r2-manifest.json

node --check scripts/docs-site/r2-upload.mjs exits 0.

The hit-without-shape-check has been present since the R2 publish path landed in d6e2734 (2026-05-07). File-path hits were added in 40cd8f4 (2026-05-16). Related publish work: #74 , #139 , #143 .

Real behavior proof

  • Behavior or issue addressed: A 200 remote manifest of {}, null, or a JSON array was treated as a hit, so a scoped upload rewrote the remote catalog from an empty entry set.
  • Real environment tested: Windows 11, Node v24.19.0, repo checkout at C:\Users\sebta.grok\tmp\pr-gate-batch\docs-f001 on branch fix/r2-empty-manifest-hit, synthetic R2 endpoint https://synthetic-r2.invalid plus a file-path remote manifest.
  • Exact steps or command run after this patch: node C:\Users\sebta\AppData\Local\Temp\r2-empty-manifest-driver.mjs C:\Users\sebta.grok\tmp\pr-gate-batch\docs-f001 . The driver writes a one-entry local manifest, then runs scripts/docs-site/r2-upload.mjs for a file-path {} with R2_UPLOAD_SCOPE=locale, an HTTP 200 {}, and an HTTP 200 { version: 1, entries: [], objectCount: 0 }.
  • Evidence after fix: terminal output above. File-path {} and HTTP 200 {} exit 1 with remote manifest must contain an entries array; refusing to reupload the full docs tree. The valid empty entries array stays a hit and completes the upload plan.
  • Observed result after fix: Empty or wrong-shape remote bodies no longer stamp status hit. The locale-scoped run does not write docs-r2-manifest.locale.merged.json. A real empty catalog (entries: []) still publishes.
  • What was not tested: A live Cloudflare R2 bucket with production credentials. Invalid JSON text still uses the existing parse error path and was not re-exercised here.

Require Array.isArray(parsed.entries) before treating an R2 or
file-path remote manifest as a hit. Empty objects, null, and JSON
arrays now fail closed so a scoped upload cannot rewrite the remote
catalog from an empty set. Valid empty entries arrays still hit.
Dry-run missing and HTTP 404 stay missing.

Signed-off-by: Sebastien Tardif <SebTardif@ncf.ca>
@steipete

steipete commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Verified head 9e717a2710de09ed4f56794c48ad7cddcaf4ee9a.

Ran the production node scripts/docs-site/r2-upload.mjs against a loopback HTTP server with synthetic credentials and a locale-scoped manifest. On origin/main, a remote {} catalog exited successfully and sent two PUTs. This PR rejects {}, null, [], and {"entries":{}} with exit 1 before any PUT. A valid catalog succeeds and retains the out-of-scope fr/retained entry alongside de/page.

All 12 node --test scripts/docs-site/r2-upload.test.mjs regressions passed. Branch autoreview against origin/main, through P2, returned scoped-clean.

Exact-head checks:

Both workflows succeeded after approval of the pending fork runs. Ready for the orchestrator's squash merge. Contributor credit is preserved; the shared changelog update will be in #167.

@steipete
steipete merged commit a0022db into openclaw:main Sep 5, 2026
5 checks passed
steipete pushed a commit that referenced this pull request Sep 5, 2026
Apply configurable request deadlines to hostname cutover API calls and reject malformed or overflowing budgets before requests begin.

Document both request-timeout controls and consolidate the prepared dependency and publishing changelog entries. Land after PRs #132, #152, #161, #163, #164, #165, and #166.

Co-authored-by: Sebastien Tardif <SebTardif@ncf.ca>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants