77 lines
4.4 KiB
Markdown
77 lines
4.4 KiB
Markdown
# Security Audit: Community Profile Sync
|
|
|
|
## Surface Area
|
|
|
|
This feature adds:
|
|
1. A new storage tag `Tag::CommunityMembership = 0x0E` in the users partition
|
|
2. An in-memory index `CommunityMembershipIndex` keyed by user_id
|
|
3. Three new public API methods: `join_community`, `get_community_memberships`, `is_community_member`
|
|
4. Best-effort signal forwarding from primary write path to community aggregates
|
|
|
|
---
|
|
|
|
## Threat Analysis
|
|
|
|
### Input Validation
|
|
|
|
`join_community` validates:
|
|
- Community name is non-empty (`TidalError::InvalidInput` on empty string)
|
|
- Community name byte length ≤ 64 (`TidalError::InvalidInput` on overflow)
|
|
|
|
**Finding:** The storage key suffix is bounded by the 64-byte limit. Key length = 10 bytes (entity_id + separator + tag) + ≤ 64 bytes (name) = ≤ 74 bytes total. No buffer overflow risk. No injection vector — the name is stored as raw bytes in the key suffix, not parsed as structured data.
|
|
|
|
### Community Name Namespace
|
|
|
|
Community names use the `"community::<name>"` prefix in `CohortSignalLedger`. There is no namespace collision prevention between cohort names and community names, but cohort names are derived from user metadata predicates (not user input), so collision is a configuration concern, not a security concern.
|
|
|
|
**Finding:** Low risk. The `"community::"` prefix is a naming convention, not enforced by the storage layer. An application could use `"community::jazz"` as a cohort name, which would merge cohort and community signal counts. Document the convention; enforce in application layer.
|
|
|
|
### Membership Scope
|
|
|
|
`join_community(user_id, name)` takes `user_id` as a caller-supplied `u64`. There is no authentication check inside `TidalDb` — the database is a library, not an authenticated service. The calling application is responsible for ensuring `user_id` matches the authenticated user.
|
|
|
|
**Finding:** No issue at the database layer. This is the same pattern used by all other user-scoped operations (`signal_with_context`, `add_block_creator`, etc.).
|
|
|
|
### Signal Forwarding Isolation
|
|
|
|
`try_community_forwarding` forwards signals to `CohortSignalLedger` under community keys. The forwarding is:
|
|
- Best-effort: cannot be used to fail or delay the primary write path
|
|
- Additive only: it calls `CohortSignalLedger::record()` which increments scores, never decrements
|
|
- Bounded: one record call per community membership; membership is user-controlled (only via `join_community`)
|
|
|
|
**Finding:** No amplification risk. A user can only forward to communities they joined. There is no mechanism for a user to inject signals into a community they did not join.
|
|
|
|
### Startup Rebuild
|
|
|
|
`rebuild_community_memberships` scans `Tag::CommunityMembership` keys from `users_engine`. It calls `parse_key` and then interprets the suffix as a UTF-8 string. Malformed keys are skipped silently.
|
|
|
|
**Finding:** The suffix is raw bytes stored by `join_community` which validates them as UTF-8 (community name is `&str`). At rebuild time, `String::from_utf8_lossy` or explicit UTF-8 validation would be safer than `str::from_utf8` — but since only tidalDB itself writes these keys, the risk is limited to corruption scenarios, not adversarial input.
|
|
|
|
### Memory Amplification
|
|
|
|
A single user can join an arbitrary number of communities (no limit enforced). At the signal write path, this means one `CohortSignalLedger::record` per membership. A user with 10,000 memberships would add ~10,000 DashMap inserts per signal write.
|
|
|
|
**Finding:** Medium risk for adversarial inputs. Recommend adding a `MAX_COMMUNITIES_PER_USER` cap (e.g., 1000) in `join_community`. Not blocking for this milestone — the feature is an internal engine primitive, not exposed to untrusted users directly.
|
|
|
|
---
|
|
|
|
## Summary
|
|
|
|
| Finding | Severity | Status |
|
|
|---------|----------|--------|
|
|
| Input validation (length + non-empty) | — | Implemented correctly |
|
|
| Key length bounded | — | Safe (74 bytes max) |
|
|
| No auth check in DB layer | Info | By design; app responsibility |
|
|
| Community namespace collision | Low | Document convention |
|
|
| Signal forwarding isolation | — | Safe; additive only |
|
|
| Startup UTF-8 handling | Low | Only tidalDB writes keys |
|
|
| No membership count cap | Medium | Recommend future cap; not blocking |
|
|
|
|
---
|
|
|
|
## Verdict
|
|
|
|
No blocking security issues. The feature is safe for its intended use as an embedded database primitive where the caller controls user_id. The medium-risk finding (no membership cap) should be tracked as a follow-up hardening task.
|
|
|
|
APPROVED.
|