Milestone 4 validation kit and security hardening - #12
Merged
Conversation
Ship the quiet-beta materials under docs/m4, harden DoH/pin/install/CLI against review findings, expand the deterministic suite to 76 cases, and record research notes that back the roadmap.
|
Warning Review limit reached
Next review available in: 25 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (31)
📝 WalkthroughWalkthroughThe PR updates product, security, release, roadmap, research, and Milestone 4 documentation. It also hardens DoH validation, registry checks, trust-pin persistence, installation flow, verification output, and related tests. ChangesProduct and validation
Implementation hardening
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant RegistryConfig
participant Npm
participant PinStore
CLI->>RegistryConfig: Recheck effective registry
RegistryConfig-->>CLI: Return current registry
CLI->>Npm: Install with validated registry
Npm-->>CLI: Return installation result
CLI->>PinStore: Save pin with expected existing identity
PinStore-->>CLI: Return success or divergence
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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. Comment |
…opy. Compare registries after host/path normalization, treat concurrent same-identity first-use pin writes as success, align ROADMAP DNSSEC wording with the CLI, warn when verify cannot re-read npm registry config, and wire exhaustive unit coverage into npm test.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
🧹 Nitpick comments (2)
docs/HISTORY-AUDIT.md (1)
9-9: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse an H2 heading for
Checks performed.The document title is an H1.
### Checks performedskips the H2 level. Change it to## Checks performedto keep the Markdown hierarchy valid and satisfy MD001.Proposed fix
-### Checks performed +## Checks performed🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/HISTORY-AUDIT.md` at line 9, Change the “Checks performed” heading under the document’s H1 title from an H3 to an H2 by using two leading hash characters, preserving the heading text.Source: Linters/SAST tools
docs/m4/DISCOVERY-PROTOCOL.md (1)
600-605: 🗄️ Data Integrity & Integration | 🔵 Trivial | 🏗️ Heavy liftUse participant-aware uncertainty estimates.
Wilson and exact-binomial intervals treat the 18 trials as independent. This study assigns repeated trials to only six participants. If an interval is reported, use a participant-aware paired or clustered method. Otherwise label the percentages as descriptive.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/m4/DISCOVERY-PROTOCOL.md` around lines 600 - 605, Update the “Statistical honesty” guidance to require participant-aware paired or clustered uncertainty estimates, since the 18 trials are repeated across six participants rather than independent. If no such interval is reported, explicitly label the percentages as descriptive, and avoid presenting Wilson or exact-binomial intervals as appropriate standalone estimates.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/FEATURE-CANDIDATES.md`:
- Around line 855-863: Update the weekly pivot triggers to include Gate 4
correctness alongside publisher and comprehension in both
docs/FEATURE-CANDIDATES.md lines 855-863 and docs/FEATURE-RESEARCH-REPORT.md
line 791; ensure the pivot-watch wording matches the hard-stop decision rule
while preserving the existing thresholds and actions.
In `@docs/m4/COMPREHENSION.md`:
- Around line 149-166: Clarify the decision status for passes with 5–9
completers in the Gate-level and Worked examples sections. Align the 5/5 and 4/5
examples with the document’s provisional-under-10 rule, explicitly
distinguishing a provisional pass from an actionable gate pass and applying that
rule consistently before recording results.
In `@docs/m4/CONTACT-TRACKER.md`:
- Around line 32-48: Update the Tracker table and its usage guidance to make
Gate 1 eligibility explicit and auditable: add structured fields for qualified
status, external status, exclusion status, qualification evidence, and
verification date, and define how they must be populated before counting a row
in gate totals. Do not rely on free-form Notes for these eligibility decisions,
and preserve the control-row exclusion.
In `@docs/m4/DISCOVERY-PROTOCOL.md`:
- Around line 40-48: Update the primary metrics and Gate 4 criteria in the
discovery protocol to separate live CLI validation from simulated stimulus-card
scoring. Require a minimum number of live `di verify` trials for Gate 4, or
define simulated results as a separate materials-validation gate, rather than
aggregating both trial types.
- Around line 132-140: Update the consent brief in the introductory block to
disclose all retained fields: participant_id, task_id, condition,
package_answer, abandoned, notes, and timestamp. Clarify whether participant_id
is pseudonymous, and add the study’s data retention period and access terms
before the consent statement.
In `@docs/m4/PUBLISHER-GUIDE.md`:
- Around line 22-30: Update the prerequisites wording in PUBLISHER-GUIDE.md to
clarify that Node.js and npm are unnecessary for creating the DNS TXT record,
but required for installing or running the domaininstall CLI, including the
later di verification and installation steps.
In `@docs/m4/RESULTS.md`:
- Line 34: Update the “Overall M4 status” scorecard in RESULTS.md to select “Not
started” initially instead of “In progress”; leave the status transition to “In
progress” for the documented Day 1 pre-flight process in RUNBOOK.md.
- Around line 153-173: The Final decision checklist conflicts with the Hold /
incomplete option by requiring every gate to be Pass, Fail, or waived. Update
the completion criteria around the “Final decision” section to allow Incomplete
for non-hard-stop gates and permit “Hold / incomplete” without requiring a
waiver, while preserving the requirement that hard-stop gates cannot be waived
to continue.
In `@docs/m4/RUNBOOK.md`:
- Line 30: Update the roadmap link in the runbook so its fragment matches the
renderer’s generated heading slug without relying on § characters, or add and
reference an explicit stable anchor for the “Updating the roadmap §2 and §7”
heading.
- Around line 62-64: Update the published tool installation check in the runbook
to use an explicit domaininstall version for both global npm installation and
npx execution, replacing unpinned commands with the tested @<version> form.
Record the same tested version in the beta scorecard.
In `@docs/m4/USAGE-DIARY.md`:
- Around line 158-168: Update the anecdote-counting rules in the “Allowed with
care” and “Do not” sections to deduplicate the same alleged_user across
publishers and diary entries using the user identity and date. Require that key
for anecdotal claims, or count an anecdote only when it cannot match another
anecdote or diary entry, while preserving the existing one-user/one-use limits.
In `@docs/RESEARCH-BACKLOG.md`:
- Line 125: Restore the backlog item headings in docs/RESEARCH-BACKLOG.md by
separating or removing each orphan Status row that is merged with a following
#### heading. Ensure every referenced backlog item has a valid Markdown heading
and one authoritative status field, without leaving standalone table rows
lacking headers or separators.
- Around line 66-89: Synchronize each detailed research item’s Status field with
the priority-stack statuses, including RB-DOH, RB-PIN, RB-TOCTOU, RB-PURL, and
all other referenced entries. Update the closing statement near the end of the
document to state that only open or partial research items remain, rather than
claiming all items are open.
In `@docs/research/FINDINGS-POSITIONING.md`:
- Around line 114-134: Update the “Today”/“Current” DNSSEC rows in the findings
section to use the shipped labels “DNSSEC: AD” for authenticated responses and
“DNSSEC: no AD” for unauthenticated responses. Adjust surrounding wording as
needed, and remove or mark the recommendation as already implemented rather than
proposing another label change.
In `@docs/research/FINDINGS-WINDOWS-AND-SCRIPTS.md`:
- Line 151: Update the table cell in FINDINGS-WINDOWS-AND-SCRIPTS.md containing
“O_RDONLY | undefined” so the pipe is escaped or the expression is rewritten
without a table delimiter, preserving the full no-follow explanation in rendered
Markdown.
In `@scripts/test.ts`:
- Around line 663-674: Isolate the registry recheck in the test around
assertEffectiveRegistryUnchanged by creating a temporary package root containing
a project .npmrc that sets https://registry.npmjs.org/. Pass that temporary
directory as the third argument for both the stable-registry and drift checks,
avoiding reliance on process.cwd() or user/CI npm configuration.
In `@src/doh.ts`:
- Around line 190-199: Update the response-body handling in the DoH request flow
around response.text() to read from response.body as a stream, count each
Uint8Array chunk by byteLength against MAX_DOH_BODY_BYTES, and cancel the reader
when the cap is reached. Decode the accumulated bytes consistently before
parsing, and preserve malformed(provider) handling for unavailable bodies or
read/decode failures.
---
Nitpick comments:
In `@docs/HISTORY-AUDIT.md`:
- Line 9: Change the “Checks performed” heading under the document’s H1 title
from an H3 to an H2 by using two leading hash characters, preserving the heading
text.
In `@docs/m4/DISCOVERY-PROTOCOL.md`:
- Around line 600-605: Update the “Statistical honesty” guidance to require
participant-aware paired or clustered uncertainty estimates, since the 18 trials
are repeated across six participants rather than independent. If no such
interval is reported, explicitly label the percentages as descriptive, and avoid
presenting Wilson or exact-binomial intervals as appropriate standalone
estimates.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: bef6b481-64de-4d64-ba4b-ff707c385f2c
📒 Files selected for processing (36)
CHANGELOG.mdREADME.mdROADMAP.mdSECURITY.mddocs/FEATURE-CANDIDATES.mddocs/FEATURE-RESEARCH-REPORT.mddocs/HISTORY-AUDIT.mddocs/RELEASE-CANDIDATE.mddocs/RELEASE.mddocs/RESEARCH-BACKLOG.mddocs/m4/COMPREHENSION.mddocs/m4/CONTACT-TRACKER.mddocs/m4/DISCOVERY-PROTOCOL.mddocs/m4/OUTREACH.mddocs/m4/PLACEMENT-SNIPPETS.mddocs/m4/PROSPECT-CANDIDATES.mddocs/m4/PUBLISHER-GUIDE.mddocs/m4/README.mddocs/m4/RESULTS.mddocs/m4/RUNBOOK.mddocs/m4/USAGE-DIARY.mddocs/research/FINDINGS-DOH.mddocs/research/FINDINGS-M4-DRYRUN.mddocs/research/FINDINGS-NONTTY.mddocs/research/FINDINGS-PIN-AND-TOCTOU.mddocs/research/FINDINGS-POSITIONING.mddocs/research/FINDINGS-RDAP-MAXAGE-FIRSTUSE.mddocs/research/FINDINGS-SCOPE-AND-PURL.mddocs/research/FINDINGS-WINDOWS-AND-SCRIPTS.mddocs/research/README.mdpackage.jsonscripts/test.tssrc/cli.tssrc/doh.tssrc/install.tssrc/pin.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
docs/m4/(publisher guide, outreach, trackers, discovery/comprehension/usage protocols, scorecard).di verifyvs pin continuity, resolver + DNSSEC (resolver AD) wording, stderr hints..npmrcis loaded via a package root.docs/research/, feature research backlog) that support the validation-first plan.Security / CLI highlights
src/doh.ts: max body/answer/data sizes,redirect: error, wrong-name TXT → malformedsrc/pin.ts: validate on write;savePinreturns result and refuses concurrent pin divergencesrc/install.ts:assertEffectiveRegistryUnchangedbefore spawnsrc/cli.ts: pin-aware verify, conflict listing, CAS save after install, clearer--yes/ DNSSEC copyTest plan
npm test(76 passed, 0 failed) on Windows / Node 22+di verify zuraai.xyzstill resolves the reference mappingOut of scope
di setup,resolve --json, multi-PM)Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests