fix: reconcile denylist key contract#29
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
WalkthroughThis PR implements the finalized KV denylist key namespace per ADR 0057. The content service token payload now includes an optional Sequence Diagram(s)sequenceDiagram
participant Client
participant ContentWorker
participant DENYLIST
Client->>ContentWorker: Request signed content with token
ContentWorker->>ContentWorker: verify token -> payload
ContentWorker->>ContentWorker: denylistKeysForPayload(payload)
ContentWorker->>DENYLIST: Promise.all([get(ad:...), get(rd:...), maybe get(wsd:...), maybe get(ald:...)])
alt any denylist entry present
DENYLIST-->>ContentWorker: non-null
ContentWorker-->>Client: 404 not_found
else all null
DENYLIST-->>ContentWorker: nulls
ContentWorker-->>Client: 200 content
end
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/adr/0057-kv-denylist-namespace-keys-and-write-order.md`:
- Line 29: Update the ADR so the TTL policy and the stated consistency window
agree: pick a single contract (either 90-day max TTL for denylist entries or a
15-minute accepted consistency window) and update the contradictions
accordingly; specifically edit the paragraph starting "Entries must live at
least as long as the longest content-gateway token..." and the sentence that
currently states a "15-minute accepted consistency window" so they reference the
same TTL/consistency semantics (e.g., clarify that denylist entries use a 90-day
max_ttl_seconds but the operational accepted consistency window for revocation
is 15 minutes for cache propagation, or change both to reflect a unified
short-lived content-token TTL), ensuring both phrases and any references to ADR
0056 remain consistent.
🪄 Autofix (Beta)
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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: a2534a73-9a44-45f9-a6d8-813857b2be42
📒 Files selected for processing (7)
apps/api/src/index.test.tsapps/api/src/index.tsapps/content/src/error-envelope.test.tsapps/content/src/index.test.tsapps/content/src/index.tsdocs/adr/0057-kv-denylist-namespace-keys-and-write-order.mddocs/ops/project-status.md
|
agent-paste PR preview resources were cleaned up. The pr-preview-${context.issue.number} environment is left in place; remove it from the GitHub UI if desired. |
1 participant
Summary
Reconciles the MVP denylist implementation with ADR 0057 so deletion and cleanup write the same keys that
contentreads. This keeps generic public 404 behavior while preserving the current direct signed content URL lifetime.Changes
contentdenylist reads fromartifact:*/revision:*/ unwrittencontent-token:*toad:*/rd:*, with optionalwsd:*andald:*reads only when token payload IDs exist.apiartifact denylist writes toad:{artifactId}with diagnostic JSON and the current max signed content-token TTL.Risk: MEDIUM
apidenylist writes,contentdenylist reads, worker tests, ADR/status docs.artifact:*keys and should be updated in a separate scoped change before using smoke as the denial-key assertion.Test plan
pnpm turbo run test --filter=@agent-paste/content --filter=@agent-paste/apipnpm turbo run test --filter=@agent-paste/apipnpm turbo run lint typecheck --filter=@agent-paste/apipnpm verifyCodeRabbit notes
070ddcf test: cover denylist cleanup contracts.coderabbit review --agent --base maincompleted with 0 findings.Summary by CodeRabbit
Tests
Behavior
Documentation