feat(store): scalable storage — S3 WORM, async pipeline, integrity verification, FS janitor - #123
feat(store): scalable storage — S3 WORM, async pipeline, integrity verification, FS janitor#123OkeyAmy wants to merge 15 commits into
Conversation
…docs - Fix Go module path github.com/drs-protocol/drs-verify → github.com/OkeyAmy/DRS/drs-verify (was a 404; `go get` now resolves). - Remove @okeyamy/drs-mcp-server / drs-mcp-client: Node enforcement is a documented copy-paste fail-closed gate against /verify, not an npm package (a second gate impl would only drift from the normative Go middleware). - Update dependabot/conformance/e2e/publish workflows, integration-tests, README, and docs-site to match; add live-deployment explanation page. Verified: go build/vet clean, go test 15/15, SDK 118/118, docs build clean.
…esiduals Rewrites TestAsyncStore_SameKeyConcurrent_NoSilentLoss to exercise the queue-full (default: + CompareAndDelete + OnDrop) branch via a 20ms putDelay inner store and QueueSize:2/Workers:1, so the pre-fix buggy code no longer passes. Adds three hard assertions: queueFull>0 (branch reached), dropped==queueFull (OnDrop fires for every rejection), and inner.Get=="v" after Close (accepted write is durable). Adds doc comments on AsyncStore classifying the two known residuals (transient read miss; Delete undone by queued write).
…tRetention RemoveObject on a versioned Object-Lock bucket writes a delete marker, hiding the retained version without destroying it. Delete now logs a warning and returns nil under objectLock=true. objectRetention helper exposes GetObjectRetention for the new WORM integration test, which asserts COMPLIANCE mode and retain-until ~3650 days out — a deterministic proof unconfounded by delete markers.
…r/write-outcome counters)
There was a problem hiding this comment.
Review Summary
This PR introduces a sophisticated four-tier storage architecture with async pipelines, integrity verification, and WORM compliance. The implementation demonstrates strong engineering with comprehensive documentation and safety considerations.
Critical Issue Identified
Security Vulnerability (CWE-798): S3 credentials are stored directly in the Config struct throughout the application lifecycle. For production compliance deployments handling regulated evidence, credentials should use AWS SDK credential chains (IAM roles, instance profiles) rather than long-lived plaintext credentials in memory.
Architecture Assessment
The layered design is well-executed:
- AsyncStore properly handles concurrent writes with CompareAndDelete semantics
- IntegrityStore implements fail-closed tamper detection
- S3Store correctly implements WORM via COMPLIANCE mode Object Lock
- FilesystemStore includes robust path traversal protection via regex validation
The documented residuals (transient read misses, Delete/Write race) are acceptable for content-addressed storage and honestly disclosed.
Risk Assessment
The storage layer is production-ready except for the credential handling issue. All other security properties (fail-closed integrity checks, WORM enforcement, path traversal prevention) are correctly implemented. Once credentials are externalized, this provides a solid foundation for compliance-grade evidence storage.
You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.
| S3AccessKey string | ||
| S3SecretKey string |
There was a problem hiding this comment.
🛑 Security Vulnerability: Hardcoded S3 credentials in configuration structs create CWE-798 risk. Replace with AWS SDK credential chain or environment variable references validated at runtime, not stored in memory throughout application lifecycle.1
Footnotes
-
CWE-798: Use of Hard-coded Credentials - https://cwe.mitre.org/data/definitions/798.html ↩
…CVEs minio-go/v7 (added for the S3 store) pulled in golang.org/x/crypto@0.51.0 and golang.org/x/net@0.53.0, which carry HIGH-severity advisories: x/crypto/ssh CVE-2026-39828/39829/39830/39831/39832/39835/42508/46595/46597 x/net/html CVE-2026-25681/27136 x/net/idna CVE-2026-39821 DRS imports neither x/crypto/ssh nor x/net/html — go mod why confirms 'main module does not need package' for both, and govulncheck reports 0 reachable vulnerabilities. The advisories are still cleared at the module level (Trivy is version-based, not reachability-based) by bumping to the patched releases. Verified: full suite green under -race; trivy fs on go.mod returns 0 CRITICAL/HIGH; govulncheck ./... clean (the single remaining govulncheck note is x/crypto/openpgp, an unmaintained package with no fix that we do not call).
What this does
Replaces the single in-memory store with a four-tier storage architecture that scales from development to regulated compliance deployments. Every verified delegation receipt is now persisted durably with tamper-evident reads and an optional RFC 3161 timestamp anchor.
Tiers implemented
STORE_DIRsetS3_BUCKETsetS3_BUCKET+TSA_URLsetNew packages
pkg/store/integrity.go— content-hash verification on everyGet. Key issha256:{hex}of the value. One byte changed in storage →ErrIntegrityreturned, tampered value never reaches the caller.pkg/store/async.go— non-blocking write decorator. Verification hot path returns without waiting for S3 flush. In-flightsync.Mapbuffer guarantees read-after-write correctness within the same request. Bounded queue withErrQueueFullon saturation — loud, observable, not a silent drop.pkg/store/s3.go— minio-go v7 backend. Object Lock COMPLIANCE mode.Deleteis a logged no-op under WORM — the storage API itself enforces retention. WORM proof viaGetObjectRetention, not delete-then-read.pkg/store/filesystem.go—NeverExpiresentinel (-1) disables TTL for Tier-3 paths. Background janitor sweeps expired Tier-1 entries hourly; ctx-cancelable, re-stats before remove.Composition (Tier 3)
Each layer has one job. Tier3Store anchors. AsyncStore decouples. IntegrityStore verifies. S3Store persists.
Observability
Three new Prometheus counters wired to the async pipeline:
drs_store_write_queue_dropped_totaldrs_store_flush_errors_totaldrs_store_writes_total{result="dropped|error|flushed"}Security properties
ErrQueueFullis a named, observable signal — not a silent dropS3_OBJECT_LOCK=truerequiresS3_RETENTION_DAYS > 0— a zero retention days value would computeRetainUntilDate = now, producing zero immutability; config validation rejects itIsRevokedon out-of-range status list index returns error, not(false, nil)— revocation bypass via unknown index is closedHonest residuals (documented in progress ledger)
context.Context— S3 operations usecontext.Background(), no per-request cancelS3_USE_SSL/S3_OBJECT_LOCKparse=="true"—1orTRUEsilently becomes false.tstread-back in the same request, the timestamp evidence may miss. Each piece (WORM ✓, async ✓, integrity ✓) is tested independently; the composed interaction under load is a named follow-upTested
-race-race/verify → durablehot path proven end-to-end. Receipt stored as0622/062226…a4be.jwt, WORM enforcement confirmed (Mode: COMPLIANCE, 3649 days), delete attempt rejected by storage API