another big refactor#30
Draft
flo-bit wants to merge 22 commits into
Draft
Conversation
5 tasks
tompscanlan
reviewed
May 4, 2026
Collaborator
There was a problem hiding this comment.
Isn't the contrail-appview a third independent role that could be run? I could see a shape for just running an indexer and consuming it from multiple services.
Owner
Author
There was a problem hiding this comment.
yeah, docs need to be updated
Bring the e2e suite back to green under the package split:
- makeSpacesConfig() wraps the old flat { type, serviceDid, resolver }
shape under authority / recordHost and generates a fresh signing key
per call. The config validator now requires spaces.authority whenever
community is set, so this is the minimum viable shape for any test
that uses spaces.
- setupCommunityContrail() wires the community module via
createCommunityIntegration({ db, config: resolveConfig(...) }) and
passes the result as communityIntegration to the Contrail constructor.
The community config field is now opaque to contrail core; routes are
registered through the integration option.
- 7 test files updated to use the helpers; 4 community tests pick up a
workspace dep on @atmo-dev/contrail-community.
35/35 e2e tests pass against devnet. No source-package changes.
test(contrail-e2e): adopt post-PR30 spaces split + community integration
Main added record-filter, identity-events, follow-feed, and lex-gen-fix features in `packages/contrail/src/core/*` that PR #30 had reorganized into the new package split. Each conflict resolved by: 1. Keeping PR #30's re-export shims in `packages/contrail/src/core/*` (these files now `export * from "@atmo-dev/contrail-appview"`) 2. Porting main's additions to the corresponding files in `packages/contrail-appview/src/core/*`, which is the new home of the actual implementation. Files re-shimmed: - backfill.ts, db/records.ts, db/schema.ts, identity.ts, jetstream.ts, persistent.ts, router/feed.ts, types.ts Ports landed in contrail-appview/src/core (or contrail-base where types and identity helpers actually live): - backfill.ts, db/records.ts, db/schema.ts, jetstream.ts, persistent.ts, router/feed.ts (+1.0K lines from main) - constellation.ts (new 181-line module) - contrail-base/src/types.ts: FeedTargetConfig, ConstellationConfig, buildFeedTargetCaps, normalizeFeedTarget, feedTargetMaxItems, CollectionConfig.{timeField, subjectField, recordFilter}, and resolveConfig defaulting `discover: false` for `app.bsky.*` - contrail-base/src/identity.ts: applyIdentityEvent (handle change handler) Test mock for `persistent.test.ts` updated to mock `@atmo-dev/contrail-base` (post-split home of identity helpers) and include both `refreshStaleIdentities` and `applyIdentityEvent`. Test status post-merge: 314 passed, 7 failed in `packages/contrail/tests/search.test.ts` — these 7 failures pre-exist on plain `origin/main` and are unrelated to the merge. No behavior changes intended beyond what main and PR #30 already deliver.
3 tasks
The recordHost.enroll endpoint accepted either an owner-signed call OR a self-attested authority call. The latter let any DID claim to be the authority for any space's URI and rebind that space's authority. Drop the self-attesting-authority branch. Require the caller to be the space owner (sa.issuer === parts.ownerDid). The owner is still free to designate any authority via body.authority — only the legitimacy of the caller changes. Updates the existing test that codified the old behavior to assert the new rejection, and adds a positive test for the canonical owner-driven split-deployment enrollment flow.
The default in-process credential verifier composed [Enrollment, Local], so a never-enrolled space still resolved its authority via LocalBindingResolver — collapsing the host-side consent layer the enrollment table is meant to provide. Default to enrollment-only. createLocalBindingResolver remains exported for explicit single-tenant wiring via options.credentialVerifier.
The PDS binding resolver trusted body.value.authority on any record
returned for the space's URI, with only a startsWith('did:') sniff. A
malformed or attacker-shaped record at the same NSID could rebind
authority — record content alone, no signature verification.
Validate before trusting:
- $type must equal the URI's type (the space-type lexicon embeds the
declaration fields per tools.atmo.space.declaration's contract)
- createdAt must be a string
- authority must match a strict did:plc | did:web regex
Fail closed on any deviation. Adds three negative tests covering each
deviation path.
Two trust assumptions in the binding layer cannot be closed at the Contrail layer alone: - DID-doc service-entry rebind: deployments using createDidDocBindingResolver inherit PLC's rotation-key authorization model. Any rotation key on the owner's account can rewrite the #atproto_space_authority service entry. - PDS app password held for managed communities: community.provision and community.adopt hold unscoped app passwords that can write any record on the user-owned PDS, including the binding declaration read by createPdsBindingResolver. Both are constraints of their respective binding sources and onboarding flows, not bugs. Document each so operators picking a binding strategy can size them against their own threat model, with pointers to the canonical issues for the upstream/protocol-level work that would close them: #38 (DID-doc), #39 (PDS app password).
merge: resolve conflicts with main
fix(spaces): owner-signed enroll, enrollment-only default binding, declaration validation
Collaborator
|
I'm currently building on top of this #31, and #44 in main...tompscanlan:contrail:feat/pr30-pr31-integration I think it should merge as is. It's a good position to make updates against. Having the split modules is worth the change. Any reason to not merge it, @flo-bit ? |
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
2 participants
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.
No description provided.