fix(markdown-core): use Object.hasOwn instead of in operator in parseFrontmatterBlock by zenglingbiao · Pull Request #99129 · openclaw/openclaw

zenglingbiao · 2026-07-02T15:08:25Z

Summary

Use Object.hasOwn() instead of the in operator in parseFrontmatterBlock() to prevent prototype chain pollution when merging line-parsed frontmatter entries.

What Problem This Solves

When YAML frontmatter contains a key named after an Object.prototype property (e.g., toString, constructor, valueOf, hasOwnProperty) with a null value, the key is silently dropped from the output.

Root cause: coerceYamlFrontmatterValue(null) returns undefined, causing the YAML parser to skip the key. The inline line parser still captures it, but the merge step at frontmatter.ts:225 uses key in merged — which traverses the prototype chain. Since "toString" in {} is true (it finds Object.prototype.toString), the inline value is also discarded, and the user's data is lost.

Code evidence: In packages/markdown-core/src/frontmatter.ts:225, the code checks if (!(key in merged)) — but merged = {} inherits from Object.prototype, so prototype keys are falsely detected as "already present".

Root Cause

The in operator checks the entire prototype chain, not just own properties
Object.hasOwn() (ES2022+) correctly checks only the object's own properties
Object.hasOwn is already used 15+ times across the codebase (src/infra/, src/param-key.ts, packages/agent-core/, etc.) — this was the one remaining in operator usage on a plain object

Why This Fix

Minimal: 1-line change, no behavioral impact on normal keys
Consistent: matches existing Object.hasOwn usage pattern in the codebase
Correct: Object.hasOwn(obj, key) is the semantically correct check for "does this object own this property"
Regression-safe: normal keys pass unchanged; only prototype keys are affected

Evidence

Before: toString: null in YAML frontmatter → output {title: "Hello"} (toString lost)
After: toString: null in YAML frontmatter → output {title: "Hello", toString: "null"} (toString preserved)
All 4 prototype keys tested: toString, constructor, valueOf, hasOwnProperty — all preserved
Normal keys regression check: passed

Real Behavior Proof

Before (on main)

$ node --import tsx proof-frontmatter.mjs
FAIL: 'toString' is an own property of the result
FAIL: 'toString' value equals "null" (got: undefined)
FAIL: 'constructor' is an own property of the result
FAIL: 'constructor' value equals "null" (got: undefined)
FAIL: 'valueOf' is an own property of the result
FAIL: 'valueOf' value equals "null" (got: undefined)
FAIL: 'hasOwnProperty' is an own property of the result
FAIL: 'hasOwnProperty' value equals "null" (got: undefined)
PASS: normal key 'title' preserved (got: "Hello")

8 FAILED

After (with fix)

$ node --import tsx proof-frontmatter.mjs
PASS: 'toString' is an own property of the result
PASS: 'toString' value equals "null" (got: "null")
PASS: 'constructor' is an own property of the result
PASS: 'constructor' value equals "null" (got: "null")
PASS: 'valueOf' is an own property of the result
PASS: 'valueOf' value equals "null" (got: "null")
PASS: 'hasOwnProperty' is an own property of the result
PASS: 'hasOwnProperty' value equals "null" (got: "null")
PASS: normal key 'title' preserved (got: "Hello")

ALL PASS

Tests and Validation

pnpm check: passed (only pre-existing npm shrinkwrap guard failure, unrelated)
Manual verification: 4 prototype keys + normal keys regression tested
Diff: 1 line changed, 1 file

…FrontmatterBlock

clawsweeper · 2026-07-02T15:21:14Z

Codex review: needs maintainer review before merge. Reviewed July 4, 2026, 1:34 AM ET / 05:34 UTC.

Summary
This PR changes markdown-core frontmatter merging to use Object.hasOwn and adds a regression test for Object.prototype-named null YAML keys.

PR surface: Source 0, Tests +22. Total +22 across 2 files.

Reproducibility: yes. Source inspection shows current main drops YAML nulls, line-parses the same keys, and then suppresses Object.prototype-named keys via key in merged; the PR body also includes before/after terminal output for that path.

Review metrics: none identified.

Merge readiness
Overall: 🦞 diamond lobster
Proof: 🦞 diamond lobster
Patch quality: 🦞 diamond lobster
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Next step before merge

[P2] No repair lane is needed because the branch already contains the focused source change and regression test; remaining action is normal maintainer landing after current-head checks.

Security
Cleared: The diff only changes a parser own-property guard and adds tests; it introduces no dependency, workflow, secret, package, or code-execution surface.

Review details

Best possible solution:

Land the narrow Object.hasOwn guard and regression test after normal current-head gates; broader null-prototype frontmatter hardening can be tracked separately if maintainers want it.

Do we have a high-confidence way to reproduce the issue?

Yes. Source inspection shows current main drops YAML nulls, line-parses the same keys, and then suppresses Object.prototype-named keys via key in merged; the PR body also includes before/after terminal output for that path.

Is this the best way to solve the issue?

Yes. Object.hasOwn is the narrowest maintainable fix for this merge fallback because the decision is whether merged owns the key; null-prototype parser maps would be broader hardening outside this reported path.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against b68e2e701ae5.

Label changes

Label justifications:

P2: This is a normal-priority correctness fix for an edge-case markdown frontmatter data-loss path with limited blast radius.
rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body includes copied before/after terminal output showing the parser result change from dropped prototype-named null keys to preserved string values.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes copied before/after terminal output showing the parser result change from dropped prototype-named null keys to preserved string values.

Evidence reviewed

PR surface:

Source 0, Tests +22. Total +22 across 2 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	1	1	0
Tests	1	22	0	+22
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	2	23	1	+22

What I checked:

Current main bug path: Current main drops YAML null values in coerceYamlFrontmatterValue, line-parses the same inline null entries, then uses key in merged at the fallback merge point, so inherited Object.prototype names can be treated as already present. (packages/markdown-core/src/frontmatter.ts:225, b68e2e701ae5)
PR patch targets implicated guard: The PR changes only the fallback merge check from !(key in merged) to !Object.hasOwn(merged, key), which matches the needed own-property question at this merge boundary. (packages/markdown-core/src/frontmatter.ts:225, adb2758df903)
Regression coverage: The added test covers toString, constructor, valueOf, and hasOwnProperty with YAML null values and checks that a normal title key remains preserved. (packages/markdown-core/src/frontmatter.test.ts:103, adb2758df903)
Caller surface checked: Skill loading, hook loading, and bundled command loading all consume parseFrontmatterBlock, so fixing the shared parser is the right boundary instead of duplicating caller policy. (src/skills/loading/frontmatter.ts:25, b68e2e701ae5)
YAML dependency contract inspected: [email protected] parses via doc.toJS, resolves YAML nulls to JavaScript null, and writes object map entries as enumerable properties, while OpenClaw then intentionally skips nulls before the line-parser fallback.
Maintainer validation context: A member comment reports source reproduction, YAML 2.9.0 inspection, focused frontmatter test success, fresh autoreview with no actionable findings, and hosted gates on exact head 09508c5fc148c8ba7a2b1cb06c3dff5add437e51; the current head is a later merge-from-main commit with the same PR diff. (adb2758df903)

Likely related people:

steipete: Git history shows the earlier frontmatter parser was introduced by Peter Steinberger in 1e2ab8bf1e, and merged PR feat(providers): add ClawRouter routing and quotas #99658 brought the current markdown-core frontmatter package files onto main. (role: introduced current parser behavior and recent merger; confidence: high; commits: 1e2ab8bf1ed0, 4a354f76c1bf; files: packages/markdown-core/src/frontmatter.ts, packages/markdown-core/src/frontmatter.test.ts, src/markdown/frontmatter.ts)
vincentkoc: Vincent Koc posted the maintainer validation comment and authored the merge-from-main commit on this PR branch after the exact-head review. (role: recent reviewer and current-head updater; confidence: medium; commits: adb2758df903, e085fa1a3ffd; files: packages/markdown-core/src/frontmatter.ts, packages/markdown-core/src/frontmatter.test.ts)
zenglingbiao: Beyond opening this PR, zenglingbiao authored the related merged own-property fix in fix(config): use Object.hasOwn instead of in operator in restoreOriginalValueOrThrow #99152, making them relevant to this specific guard pattern. (role: adjacent same-pattern contributor; confidence: medium; commits: 5519c17a2984, 5bbae9ab5c39, 459b93e3ddaf; files: src/config/redact-snapshot.ts, src/config/redact-snapshot.test.ts, packages/markdown-core/src/frontmatter.ts)

What the crustacean ranks mean

🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works

ClawSweeper keeps one durable marker-backed review comment per issue or PR.
Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
Maintainers can also comment @clawsweeper review to request a fresh review only.
Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

…ntmatter keys

zenglingbiao · 2026-07-02T15:32:16Z

Added a focused regression test for prototype-named null keys as suggested by the review. The test covers toString, constructor, valueOf, and hasOwnProperty with null YAML values — all now correctly preserved by Object.hasOwn.

@clawsweeper re-review

clawsweeper · 2026-07-02T15:32:20Z

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

zenglingbiao · 2026-07-02T23:34:13Z

@clawsweeper re-review

clawsweeper · 2026-07-02T23:34:16Z

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

zenglingbiao · 2026-07-04T05:05:03Z

@clawsweeper re-review

vincentkoc · 2026-07-04T05:28:31Z

maintainer review is complete on exact head 09508c5fc148c8ba7a2b1cb06c3dff5add437e51.

validation:

reproduced the current-main inherited-property failure path by source inspection
inspected YAML 2.9.0 source and runtime behavior: prototype-named YAML keys are own enumerable properties, while OpenClaw intentionally drops null values before the line-parser fallback
node scripts/run-vitest.mjs packages/markdown-core/src/frontmatter.test.ts (10 passed)
fresh autoreview: no accepted/actionable findings
exact-head hosted CI/Testbox gates passed through scripts/pr prepare-run

best-fix verdict: Object.hasOwn is the narrowest correct check at the shared markdown-core merge boundary. no maintainer repair is needed.

vincentkoc · 2026-07-04T05:39:30Z

Merged via squash.

Prepared head SHA: adb2758df9031675cf10eaedf85add8185a2f8fb
Landed commit: acdaf8ae31e6e10d10b26c1e9d2bf0a6cea3ec70

…FrontmatterBlock (openclaw#99129) * fix(markdown-core): use Object.hasOwn instead of in operator in parseFrontmatterBlock * test(markdown-core): add regression test for prototype-named null frontmatter keys * style(markdown-core): fix unbound-method lint in regression test --------- Co-authored-by: Vincent Koc <[email protected]>

…obIdentityFields Replace "jobId" in raw with Object.hasOwn(raw, "jobId") to prevent prototype chain pollution when migrating legacy cron job identity fields. The in operator traverses the prototype chain, so a polluted Object.prototype.jobId would cause the function to incorrectly detect a legacy jobId key and attempt to delete it from the row. This is the third fix in the series after openclaw#99152 and openclaw#99129.

fix(markdown-core): use Object.hasOwn instead of in operator in parse…

a8ec531

…FrontmatterBlock

openclaw-barnacle Bot added the size: XS label Jul 2, 2026

test(markdown-core): add regression test for prototype-named null fro…

1a9e3f6

…ntmatter keys

zenglingbiao mentioned this pull request Jul 2, 2026

fix(config): use Object.hasOwn instead of in operator in unsetConfigValueAtPath #99148

Closed

style(markdown-core): fix unbound-method lint in regression test

09508c5

zenglingbiao mentioned this pull request Jul 2, 2026

fix(config): use Object.hasOwn instead of in operator in restoreOriginalValueOrThrow #99152

Merged

clawsweeper Bot added status: 🔁 re-review loop A fresh ClawSweeper review was explicitly requested after the latest review. and removed status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Jul 2, 2026

clawsweeper Bot added status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed status: 🔁 re-review loop A fresh ClawSweeper review was explicitly requested after the latest review. labels Jul 2, 2026

openclaw-barnacle Bot added triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. size: S and removed size: XS triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. labels Jul 4, 2026

zenglingbiao force-pushed the fix/prototype-pollution-in-frontmatter branch from 7f17546 to 09508c5 Compare July 4, 2026 05:03

openclaw-barnacle Bot added size: XS and removed size: S labels Jul 4, 2026

vincentkoc self-assigned this Jul 4, 2026

Merge branch 'main' into fix/prototype-pollution-in-frontmatter

adb2758

zenglingbiao mentioned this pull request Jul 4, 2026

fix(config): use Object.hasOwn instead of in operator in unsetConfigValueAtPath #99826

Closed

vincentkoc merged commit acdaf8a into openclaw:main Jul 4, 2026
80 checks passed

zenglingbiao mentioned this pull request Jul 4, 2026

fix(config): use Object.hasOwn instead of in operator for provider validation #99849

Closed

github-actions Bot mentioned this pull request Jul 4, 2026

📡 Upstream Digest — 2026-07-04 06:40 UTC curtismercier/openclaw-mods#1202

Open

This was referenced Jul 7, 2026

[Bug]: Markdown frontmatter parser treats non-delimiter --- prefixes as metadata fences #101769

Closed

fix: require exact markdown frontmatter delimiters #101797

Closed

fix: require exact markdown frontmatter fences #101803

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(markdown-core): use Object.hasOwn instead of in operator in parseFrontmatterBlock#99129

fix(markdown-core): use Object.hasOwn instead of in operator in parseFrontmatterBlock#99129
vincentkoc merged 4 commits into
openclaw:mainfrom
zenglingbiao:fix/prototype-pollution-in-frontmatter

zenglingbiao commented Jul 2, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented Jul 2, 2026 •

edited

Loading

Uh oh!

zenglingbiao commented Jul 2, 2026

Uh oh!

clawsweeper Bot commented Jul 2, 2026 •

edited

Loading

Uh oh!

zenglingbiao commented Jul 2, 2026

Uh oh!

clawsweeper Bot commented Jul 2, 2026 •

edited

Loading

Uh oh!

zenglingbiao commented Jul 4, 2026

Uh oh!

vincentkoc commented Jul 4, 2026

Uh oh!

Uh oh!

vincentkoc commented Jul 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

zenglingbiao commented Jul 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

What Problem This Solves

Root Cause

Why This Fix

Evidence

Real Behavior Proof

Before (on main)

After (with fix)

Tests and Validation

Uh oh!

clawsweeper Bot commented Jul 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zenglingbiao commented Jul 2, 2026

Uh oh!

clawsweeper Bot commented Jul 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zenglingbiao commented Jul 2, 2026

Uh oh!

clawsweeper Bot commented Jul 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zenglingbiao commented Jul 4, 2026

Uh oh!

vincentkoc commented Jul 4, 2026

Uh oh!

Uh oh!

vincentkoc commented Jul 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

zenglingbiao commented Jul 2, 2026 •

edited

Loading

clawsweeper Bot commented Jul 2, 2026 •

edited

Loading

clawsweeper Bot commented Jul 2, 2026 •

edited

Loading

clawsweeper Bot commented Jul 2, 2026 •

edited

Loading