-
Notifications
You must be signed in to change notification settings - Fork 39
fix(node): reconcile public-read messaging with per-repo enforcement #439
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
|
|
@@ -95,7 +95,7 @@ Live-network blockers to prioritize: | |||
| - GraphQL POST is still open for compatibility; GraphQL mutations should get mutation-aware auth before it becomes a public write API surface. | ||||
| - Push authorization is still not capability-complete. A valid DID signature is authentication, not authorization. Owner checks are now enforced on every branch, protected or not (`GITLAWB_ENFORCE_OWNER_PUSH`, on by default); what remains is that a UCAN `git/push` capability is not yet honored, so a delegated or CI key cannot push. | ||||
| - UCAN chain validation is incomplete and UCAN revocation/blocklisting is not implemented as an operator feature. | ||||
| - Private repository reads are not enforced. `is_public` and `GITLAWB_PUBLIC_READ` exist, but per-repository private-read behavior is not wired. | ||||
| - Private repository reads are enforced per repository through `is_public` and path-scoped visibility rules. `GITLAWB_PUBLIC_READ` remains reserved and inert. | ||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This entry remains under “Live-network blockers to prioritize,” but the revised text now says private-read enforcement is implemented and the remaining flag is intentionally inert. Keeping this resolved, non-actionable statement in the blocker list makes the readiness audit ambiguous; remove it or move it to a section that records implemented controls.
Suggested change
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time! |
||||
| - Peer URLs are self-asserted by DIDs. Signatures prove control of the DID key when present, not ownership/safety of the announced URL. | ||||
| - Outbound peer fetch/ping/sync paths should be reviewed for SSRF protections before accepting arbitrary public peer registrations. | ||||
|
|
||||
|
|
@@ -146,7 +146,7 @@ Risks: | |||
| ## Obvious live-network priorities | ||||
|
|
||||
| 1. Implement repo write authorization: owner checks, UCAN capability checks, and clear delegation semantics for push/PR/issue/bounty operations. | ||||
| 2. Implement private-read enforcement or remove private repo affordances until it exists. | ||||
| 2. Private reads are already enforced per repository (`is_public` plus path-scoped rules); retire the reserved and inert `GITLAWB_PUBLIC_READ` flag or document it as permanently reserved. | ||||
| 3. Add UCAN revocation/blocklisting and operator docs for emergency key compromise. | ||||
| 4. Harden peer registration and outbound fetch behavior against SSRF and peer-list poisoning. | ||||
| 5. Add Docker/installer/release smoke tests to CI. | ||||
|
|
||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test says it must ensure operators are directed to the real per-repository control, but it only requires “inert” and forbids the exact phrase “not wired.” Removing every reference to
is_publicand path-scoped visibility—or describing missing enforcement with different words—would leave the test green. Assert the substantive guidance so this correction remains protected.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!