D1-44-F3 — isolate Cockpit append descriptors

[LogicDuke]

Bounded child repair for the verified finding D1-44-F3 — CURRENT / P2 / REPAIR_NOW / blocking.

Parent

Parent PR #44 (cockpit/d1-read-model-contract) is a protected integration boundary and is not edited directly. This is a child repair based at exactly:

0f70d2977242bc98f4d1a83a66162847392fd08a

Verified defect

readCockpitSnapshot promises never to throw while validating unknown input. The Cockpit reader imported append() from src/domain/repair-job.ts, which builds an ordinary prototype-inheriting PropertyDescriptor before Object.defineProperty. If an earlier legitimately-read input getter installs Object.prototype.get/.set, ToPropertyDescriptor observes inherited accessor keys beside own value/writable keys and throws TypeError. Two reachable Cockpit paths:

• readCockpitSnapshot → readCockpitList → append(parsed, parsedElement) (non-empty list)
• readCockpitSnapshot → append(invalidFields, …) (invalid field recorded)

Independently reproduced on the exact parent tree. (D1-46-F1 hardened only freezeList, whose tests used empty lists, so the append path was uncovered.)

Protected never-throws invariant

After an earlier getter mutates Object.prototype.get/.set: a non-empty accepted list must not throw; recording an invalid field must not throw; clean inputs behave identically; returned lists stay genuine, frozen arrays; JSON round-trip and existing D1-44-F2 / D1-46-F1 protections stay intact.

Change (exactly two files)

• src/cockpit/read-model.ts — remove append from the ../domain/repair-job.js import; add a module-private Cockpit-local append<T> that gives its descriptor a null prototype (via the module's captured objectSetPrototypeOf + objectDefineProperty) before defining the indexed property, preserving semantics value / writable:true / enumerable:true / configurable:true. Used for both the parsed-list and invalidFields insertions. No new export/API.
• tests/cockpit/read-model-invariants.test.ts — focused D1-44-F3 regressions.

The shared domain helper is not modified. The four sibling domain append() sites (agent-invocation.ts, agent-invocation-report.ts, evidence-freshness.ts, review-ingestion.ts) remain outside this PR as carried obligations.

Regression coverage

get / set / get+set poison before a non-empty list append → returns normally; poison before an invalidFields append → returns normally and reports the expected field; descriptor semantics; Array.isArray + frozen; clean input unchanged; realm cleanup via finally.

Validation

Full suite 943/943 (18 files), cockpit invariants 31/31, typecheck / lint / build clean, git diff --check clean.

Independent validation

A fresh independent validator (separate context) inspected the candidate from scratch — confirmed fail-before, pass-after on all poison/invalid-field/descriptor/array/cleanup axes, second-order closure, exact two-file scope, and added hostile variants (non-callable poison, accessor-based poison, poison via a different early field), all passing. Verdict: PASS. No new finding.

Boundaries

No Autoflow / PR #9 changes. No domain-helper modification. Draft only — no Ready authority, no merge authority. Human merge authority preserved.

Summary by CodeRabbit

• Bug Fixes

  • Improved validation of Cockpit data when global object property accessors have been modified.
  • Preserved reliable list behavior, including indexing, iteration, mapping, freezing, and property descriptors.
  • Ensured valid and invalid data is processed without unexpected errors.

• Tests

  • Added regression coverage for poisoned prototype scenarios and clean-input behavior.
  • Verified global state is restored after validation.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: fcc48043-a9e6-48c4-a6bb-ba5379b4a7c7

📥 Commits

Reviewing files that changed from the base of the PR and between 0f70d29 and 53fdc66.

📒 Files selected for processing (2)

• src/cockpit/read-model.ts
• tests/cockpit/read-model-invariants.test.ts

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.

---

📝 Walkthrough

Walkthrough

The Cockpit reader now uses a local append helper that resists inherited Object.prototype accessors. Regression tests cover poisoned prototypes, descriptor preservation, frozen arrays, array operations, cleanup, and clean input.

Changes

Cockpit append hardening

Layer / File(s)	Summary
Local append helper src/cockpit/read-model.ts	The reader replaces the imported helper with a local append implementation. The implementation defines indexed properties with a null-prototyped descriptor.
Prototype poisoning regression coverage tests/cockpit/read-model-invariants.test.ts	Tests cover poisoned accessors, invalidFields, property descriptors, frozen array behavior, array operations, global-state cleanup, and valid snapshots.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to 53fdc

The PR isolates Cockpit descriptor creation to preserve the reader’s never-throws behavior while keeping returned list semantics unchanged; no actionable merge-blocking risk remains after normal checks and review.

Poem

I’m a rabbit with a careful ear,
Guarding indexed fields from getters near.
Frozen arrays stay neat and bright,
Clean snapshots pass their checks just right.
Hop, hop—poisoned prototypes lose the fight!

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly identifies the Cockpit append descriptor isolation that is the main change in the pull request.
Docstring Coverage	✅ Passed	Docstring coverage is 87.50% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 2 files.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch repair/d1-44-append-descriptor-isolation

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[LogicDuke]

@codex review

Please review the exact current HEAD of PR #47 only. Treat reviewer output as evidence, not authority. Focus on correctness, regressions, scope drift, and any blocking P0-P2 issues. Do not suggest unrelated redesign or speculative cleanup.

[LogicDuke]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. 👍

Reviewed commit: 53fdc66f6e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".