Fix/nip alignment 20260215#2
Merged
EmmanuelAlmonte merged 25 commits intosecurity-hardeningfrom Feb 28, 2026
Merged
Conversation
Add regressions for kind 17375 replaceable conflict handling and whitelist non-exempt blocking while wallet exempt kinds remain allowed. Add runtime NIP-11 supported_nips assertion for NIP-60 and introduce a CI migration upgrade-downgrade-reupgrade workflow.
There was a problem hiding this comment.
Pull request overview
Aligns relay behavior and tests with multiple NIP semantics (subscriptions, search, AUTH, zap handling, vanish replay hardening) and stabilizes the test suite via harness improvements and new conformance coverage.
Changes:
- Tightens protocol validation and semantics (subscription id rules, filter hex validation, COUNT distinct semantics, NIP-50 search extensions, NIP-59/78/62 behaviors).
- Improves subscription/event delivery behavior and resource handling (bounded queues for slow consumers, rate-limit exemptions).
- Expands/updates automated coverage (new integration/unit tests plus SpecFlow scenario adjustments and new feature files).
Reviewed changes
Copilot reviewed 114 out of 286 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| test/Netstr.Tests/WhitelistTests.cs | Updates whitelist tests to use local factories and finalized events. |
| test/Netstr.Tests/WebApplicationFactory.cs | Adds test-stability limits and a filter feature flag injection. |
| test/Netstr.Tests/Subscriptions/SubscriptionTests.cs | Adds invalid/uppercase filter rejection tests. |
| test/Netstr.Tests/Subscriptions/SubscriptionFilterMatcherTests.cs | New unit tests for single-element tag robustness. |
| test/Netstr.Tests/Subscriptions/SearchMatcherTests.cs | New unit tests for search extension semantics. |
| test/Netstr.Tests/Subscriptions/MatchingExtensionsTests.cs | New DB query semantics tests for protected filters. |
| test/Netstr.Tests/Subscriptions/AndTagFiltersTests.cs | New tests for enabling/disabling AND tag filters. |
| test/Netstr.Tests/SubscriptionIdContractTests.cs | New contract tests for empty/too-long subscription ids. |
| test/Netstr.Tests/SearchSemanticsIntegrationTests.cs | New integration tests for NIP-50 semantics across stored/realtime. |
| test/Netstr.Tests/SearchQueryParserTests.cs | New unit tests for parsing basic terms vs extensions. |
| test/Netstr.Tests/RateLimitingTests.cs | Refactors and adds coverage for rate-limit exemptions. |
| test/Netstr.Tests/Nip62ReplayHardeningTests.cs | New integration test for vanish/giftwrap replay rejection. |
| test/Netstr.Tests/Nip59And78ConformanceTests.cs | New integration tests for kind 13 and 30078 validation. |
| test/Netstr.Tests/NegentropyTests.cs | Adds multi-filter NEG-OPEN and invalid shape coverage. |
| test/Netstr.Tests/NIPs/Types.cs | Enhances SpecFlow client message capture (NOTICE/events, richer OK/CLOSED). |
| test/Netstr.Tests/NIPs/Transforms.cs | Improves SpecFlow transforms and event finalization logic. |
| test/Netstr.Tests/NIPs/Steps/64.cs | Adds SpecFlow steps for NIP-64 scenarios and event assertions. |
| test/Netstr.Tests/NIPs/Steps/17.cs | Adds SpecFlow steps for kind 10050 relay list semantics. |
| test/Netstr.Tests/NIPs/Steps/11.cs | Fixes HTTP request step to target configured websockets path. |
| test/Netstr.Tests/NIPs/Steps/01.cs | Normalizes SpecFlow message assertions and adds diagnostics toggles. |
| test/Netstr.Tests/NIPs/Nip11SupportedNipsTests.cs | New tests for runtime supported_nips contents. |
| test/Netstr.Tests/NIPs/Nip11NonRootPathTests.cs | New tests for NIP-11 on non-root websockets path. |
| test/Netstr.Tests/NIPs/78.feature | New SpecFlow feature for NIP-78 scenarios. |
| test/Netstr.Tests/NIPs/59.feature | New SpecFlow feature for NIP-59 scenarios. |
| test/Netstr.Tests/NIPs/57.feature | Updates zap request behavior expectations (relay rejects). |
| test/Netstr.Tests/NIPs/51.feature | Adjusts replaceable list expectations to tolerate dynamic ids. |
| test/Netstr.Tests/NIPs/50.feature | New SpecFlow feature for NIP-50 search scenarios. |
| test/Netstr.Tests/NIPs/45.feature | Stabilizes DM content/id assertions using wildcards. |
| test/Netstr.Tests/NIPs/17.feature | Fixes bad author values and adds relay-list scenarios. |
| test/Netstr.Tests/NIPs/11.feature | Improves NIP-11 expectations and adds multi-value Accept case. |
| test/Netstr.Tests/NIPs/05.feature | Switches hardcoded ids to wildcards for finalized events. |
| test/Netstr.Tests/NIPs/04.feature | Updates DM content and id expectations to match new validators. |
| test/Netstr.Tests/NIPs/02.feature | Uses wildcard ids and updates follow-list assertions accordingly. |
| test/Netstr.Tests/NIPs/01.feature | Adjusts invalid signature case and adds missing expected event. |
| test/Netstr.Tests/MultiFilterLimitSemanticsTests.cs | New integration tests for multi-filter limit behavior. |
| test/Netstr.Tests/MessageDispatcherTests.cs | Updates handler construction to match new DI signatures/options. |
| test/Netstr.Tests/MemoryLeakTest.cs | New memory pressure tests for slow-consumer queue behavior. |
| test/Netstr.Tests/LimitsTests.cs | Expands subscription id test matrix (empty id). |
| test/Netstr.Tests/Events/WhitelistValidatorTests.cs | Adds tests for exempt kinds (cashu/nutzap) behavior. |
| test/Netstr.Tests/Events/SealEventValidatorTests.cs | New unit tests for kind 13 tag rules. |
| test/Netstr.Tests/Events/ListEventValidatorTests.cs | Adds tests for 30078 d-tag and DM relay list relay-tag requirements. |
| test/Netstr.Tests/Events/EventVerificationTests.cs | Adds stub NIP-05 verification service for validator DI stability. |
| test/Netstr.Tests/Events/DbFilterEventMatchingTests.cs | Updates queries + adds wallet/nutzap kind filtering tests. |
| test/Netstr.Tests/Events/ClientContextTests.cs | New tests for multi-pubkey auth context behavior. |
| test/Netstr.Tests/Events/AuthCreatedAtValidatorTests.cs | New unit tests for AUTH created_at window enforcement. |
| test/Netstr.Tests/CountSemanticsTests.cs | New integration tests for COUNT distinct/unlimited semantics. |
| test/Netstr.Tests/ConfigurationExtensions.cs | Improves config binding for bools and enumerable flattening. |
| test/Netstr.Tests/Bob.cs | Adds deterministic test keypair helper. |
| test/Netstr.Tests/AuthTests.cs | Updates relay tag normalization expectations and adds port/slash case. |
| src/Netstr/appsettings.json | Adds auth window, updates supported NIPs, extends whitelist exempt kinds. |
| src/Netstr/appsettings.example.json | New full safe example config including Filters options. |
| src/Netstr/Views/Home/Index.cshtml | Updates hosted logo URL. |
| src/Netstr/RelayInformation/RelayInformationDefaults.cs | Updates software URL used in relay info defaults. |
| src/Netstr/Options/WhitelistOptions.cs | Adds rate-limit exemption public key list option. |
| src/Netstr/Options/FiltersOptions.cs | Adds feature flag for AND-tag filter support. |
| src/Netstr/Options/AuthOptions.cs | Adds configurable AUTH created_at tolerance window. |
| src/Netstr/Middleware/WebSocketsMiddleware.cs | Serves NIP-11 JSON on websockets path based on Accept header. |
| src/Netstr/Messaging/WebSockets/WebSocketAdapter.cs | Reduces allocation churn in websocket receive loop. |
| src/Netstr/Messaging/UserCache.cs | Tracks vanish-deleted event ids and exposes lookup. |
| src/Netstr/Messaging/Subscriptions/Validators/WhitelistSubscriptionValidator.cs | Uses multi-auth pubkeys for whitelist checks and logging. |
| src/Netstr/Messaging/Subscriptions/Validators/SubscriptionLimitsValidator.cs | Adds explicit empty subscription id validation. |
| src/Netstr/Messaging/Subscriptions/SubscriptionsAdapterFactory.cs | Passes limits into adapter for queue sizing. |
| src/Netstr/Messaging/Subscriptions/SubscriptionsAdapter.cs | Introduces bounded queue sizing for subscription adapters. |
| src/Netstr/Messaging/Subscriptions/SubscriptionFilterMatcher.cs | Prevents tag index errors on short tags during matching. |
| src/Netstr/Messaging/Subscriptions/SubscriptionAdapter.cs | Replaces unbounded queue with bounded channel + drain logic. |
| src/Netstr/Messaging/Subscriptions/SearchQueryParser.cs | New parser splitting basic terms vs extensions for NIP-50. |
| src/Netstr/Messaging/Subscriptions/SearchMatcher.cs | Ignores unsupported extensions; basic term matching behavior. |
| src/Netstr/Messaging/Models/EventTag.cs | Adds kind tag constant. |
| src/Netstr/Messaging/Models/EventKind.cs | Adds additional kind constants (DM, cashu, nutzap, etc.). |
| src/Netstr/Messaging/Models/Event.cs | Normalizes relay/auth relay parsing via shared normalizer. |
| src/Netstr/Messaging/Models/ClientContext.cs | Supports multiple authenticated pubkeys per connection. |
| src/Netstr/Messaging/Messages.cs | Adds new validation/error message constants. |
| src/Netstr/Messaging/MessageHandlers/SubscribeMessageHandler.cs | Queries stored events using multi-auth public keys. |
| src/Netstr/Messaging/MessageHandlers/Negentropy/NegentropyOpenHandler.cs | Adds array-of-filters parsing for NEG-OPEN. |
| src/Netstr/Messaging/MessageHandlers/FilterMessageHandlerBase.cs | Adds filter validation, AND-tag feature flag, search query options. |
| src/Netstr/Messaging/MessageHandlers/EventMessageHandler.cs | Adds rate-limit exemption via whitelist options. |
| src/Netstr/Messaging/MessageHandlers/CountMessageHandler.cs | Counts distinct events and avoids MaxInitialLimit truncation. |
| src/Netstr/Messaging/MessageHandlers/AuthMessageHandler.cs | Improves relay tag normalization (ports/trailing slashes). |
| src/Netstr/Messaging/Events/Validators/ZapEventValidator.cs | Enforces zap request rejection for relay publish. |
| src/Netstr/Messaging/Events/Validators/UserVanishedValidator.cs | Rejects republishing vanished-deleted events by id. |
| src/Netstr/Messaging/Events/Validators/SealEventValidator.cs | New validator for kind 13 tag-empty requirement. |
| src/Netstr/Messaging/Events/Validators/RelayListValidator.cs | Fixes kind comparison style/semantics. |
| src/Netstr/Messaging/Events/Validators/RelayListEventValidator.cs | Fixes kind comparison style/semantics. |
| src/Netstr/Messaging/Events/Validators/ProtectedEventValidator.cs | Uses multi-auth context for protected event validation. |
| src/Netstr/Messaging/Events/Validators/Nip04DirectMessageValidator.cs | New validator for kind 4 recipient tag and content format. |
| src/Netstr/Messaging/Events/Validators/ListEventValidator.cs | Narrows set kinds + enforces relay tags for kind 10050. |
| src/Netstr/Messaging/Events/Validators/AuthCreatedAtValidator.cs | New validator for AUTH created_at tolerance window. |
| src/Netstr/Messaging/Events/Handlers/ZapEventHandler.cs | Rejects zap requests at handler level; simplifies duplicate checks. |
| src/Netstr/Messaging/Events/Handlers/VanishEventHandler.cs | Tracks deleted ids for replay protection and normalizes relay tags. |
| src/Netstr/Messaging/Events/Handlers/Replaceable/ReplaceableEventHandlerBase.cs | Adds deterministic tie-break behavior for same timestamps. |
| src/Netstr/Messaging/Events/Handlers/EventHandlerBase.cs | Corrects protected-event broadcast logic for multi-auth and recipients. |
| src/Netstr/Messaging/Events/Handlers/DeleteEventHandler.cs | Adds reference validation and cashu token delete marker enforcement. |
| src/Netstr/Messaging/Events/DbExtensions.cs | Adds provider-aware search filtering and search-quality ordering. |
| src/Netstr/Extensions/OptionsExtensions.cs | Registers Filters options. |
| src/Netstr/Extensions/MessagingExtensions.cs | Registers new validators (AUTH time, seal, nip04). |
| src/Netstr/Extensions/HttpExtensions.cs | Adds robust relay URL normalization (scheme/port/path handling). |
| src/Netstr/Data/Migrations/NetstrDbContextModelSnapshot.cs | Reflects new EventJson column. |
| src/Netstr/Data/Migrations/20260221020205_LocalModelSync.cs | Adds EventJson column migration. |
| src/Netstr/Data/EventEntity.cs | Adds EventJson storage column. |
| src/Netstr/Data/EntityMapping.cs | Populates EventJson for persisted events. |
| src/Netstr/Controllers/HomeController.cs | Removes NIP-11 Accept behavior (handled by middleware now). |
| scripts/probe-relay.ps1 | New utility to probe REQ/COUNT behavior. |
| scripts/pre-commit | New local hook to block secrets/sensitive files. |
| scripts/check-no-connection-secrets.ps1 | New CI/local check for DB password leakage in appsettings. |
| docs/test-stabilization-baseline.md | Documents test stabilization baseline and causes. |
| docs/nip-validation-2026-02-16.md | Documents validation audit and coverage snapshot. |
| docs/Whitelist.md | Updates whitelist exempt kinds guidance. |
| README.md | Updates repo badges, supported NIPs list, and setup guidance. |
| AGENTS.md | Adds repo contribution/build/test guidelines. |
| .github/workflows/secret-guard.yml | Adds workflow to detect hardcoded DB passwords. |
| .github/workflows/migration-safety.yml | Adds upgrade/downgrade migration roundtrip validation. |
Files not reviewed (22)
- src/Netstr/Data/Migrations/20260221020205_LocalModelSync.Designer.cs: Language not supported
- test/Netstr.Tests/NIPs/01.feature.cs: Language not supported
- test/Netstr.Tests/NIPs/02.feature.cs: Language not supported
- test/Netstr.Tests/NIPs/04.feature.cs: Language not supported
- test/Netstr.Tests/NIPs/05.feature.cs: Language not supported
- test/Netstr.Tests/NIPs/11.feature.cs: Language not supported
- test/Netstr.Tests/NIPs/119.feature.cs: Language not supported
- test/Netstr.Tests/NIPs/13.feature.cs: Language not supported
- test/Netstr.Tests/NIPs/17.feature.cs: Language not supported
- test/Netstr.Tests/NIPs/40.feature.cs: Language not supported
- test/Netstr.Tests/NIPs/42.feature.cs: Language not supported
- test/Netstr.Tests/NIPs/45.feature.cs: Language not supported
- test/Netstr.Tests/NIPs/50.feature.cs: Language not supported
- test/Netstr.Tests/NIPs/51.feature.cs: Language not supported
- test/Netstr.Tests/NIPs/57.feature.cs: Language not supported
- test/Netstr.Tests/NIPs/59.feature.cs: Language not supported
- test/Netstr.Tests/NIPs/62.feature.cs: Language not supported
- test/Netstr.Tests/NIPs/64.feature.cs: Language not supported
- test/Netstr.Tests/NIPs/65.feature.cs: Language not supported
- test/Netstr.Tests/NIPs/70.feature.cs: Language not supported
- test/Netstr.Tests/NIPs/77.feature.cs: Language not supported
- test/Netstr.Tests/NIPs/78.feature.cs: Language not supported
Comments suppressed due to low confidence (1)
src/Netstr/Data/EntityMapping.cs:1
- Persisting
EventJson = JsonSerializer.Serialize(e)for every event adds non-trivial CPU and storage overhead (especially since event fields already exist in structured columns). If this is only needed for debugging/auditing, consider making it optional (feature-flagged), using a cheaper representation, or documenting and justifying the added storage cost (including expected DB growth).
using Netstr.Messaging.Models;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| object[] msg = message[0].GetString() switch | ||
| { | ||
| MessageType.Event => [message[2].DeserializeRequired<EventId>().Id], | ||
| MessageType.Ok => [message[2].GetBoolean(), ""], | ||
| MessageType.Closed => [""], | ||
| MessageType.Ok => [message[2].GetBoolean(), message[3].GetString() ?? string.Empty], |
There was a problem hiding this comment.
AddReceivedMessage assumes OK frames always have a 4th element (message[3]). In this PR there are tests reading OK messages defensively (Length > 3), so an OK frame with only 3 elements would throw IndexOutOfRangeException here. Fix by guarding on message.Length when reading message[3] (e.g., default message text to empty when missing).
Suggested change
| MessageType.Ok => [message[2].GetBoolean(), message[3].GetString() ?? string.Empty], | |
| MessageType.Ok => [message[2].GetBoolean(), message.Length > 3 ? message[3].GetString() ?? string.Empty : string.Empty], |
Comment on lines
+26
to
+30
| if (string.IsNullOrEmpty(id)) | ||
| { | ||
| return Messages.InvalidSubscriptionIdEmpty; | ||
| } | ||
| else if (limits.MaxSubscriptionIdLength > 0 && id.Length > limits.MaxSubscriptionIdLength) |
There was a problem hiding this comment.
Empty subscription ids are rejected, but whitespace-only ids (e.g. " ") still pass IsNullOrEmpty and will be accepted. If whitespace-only ids are considered invalid (consistent with “empty”), prefer string.IsNullOrWhiteSpace(id) to avoid accepting ids that are effectively blank.
| if (!relays.Any(x => x == path || x == AllRelaysValue)) | ||
| { | ||
| throw new EventProcessingException(e, string.Format(Messages.InvalidWrongTagValue, EventTag.Relay)); | ||
| sender.SendNotOk(e.Id, string.Format(Messages.InvalidWrongTagValue, EventTag.AuthRelay)); |
There was a problem hiding this comment.
The relay-tag validation failure message now formats the error with EventTag.AuthRelay, but the check accepts values derived from both relay and auth_relay tags. This can mislead clients debugging why the vanish was rejected. Consider referencing EventTag.Relay (as before) or producing a message that mentions both relay and auth_relay.
Suggested change
| sender.SendNotOk(e.Id, string.Format(Messages.InvalidWrongTagValue, EventTag.AuthRelay)); | |
| sender.SendNotOk(e.Id, string.Format(Messages.InvalidWrongTagValue, $"{EventTag.Relay}/{EventTag.AuthRelay}")); |
Comment on lines
22
to
+23
| private readonly ConcurrentDictionary<string, User> users = new(); | ||
|
|
||
| public User? GetByPublicKey(string publicKey) | ||
| private readonly ConcurrentDictionary<string, byte> vanishDeletedEventIds = new(StringComparer.Ordinal); |
There was a problem hiding this comment.
vanishDeletedEventIds grows monotonically and is never evicted. A user vanishing with a large history (or repeated vanishes across many users) can lead to unbounded memory growth over time. Consider bounding this structure (e.g., time-based expiration via MemoryCache, a size cap + eviction, or persisting/querying a durable marker in the DB instead of keeping all ids indefinitely in-memory).
Comment on lines
+48
to
62
| public void TrackVanishDeletedEvents(IEnumerable<string> eventIds) | ||
| { | ||
| foreach (var user in users) | ||
| foreach (var eventId in eventIds) | ||
| { | ||
| this.users.TryAdd(user.PublicKey, user); | ||
| if (!string.IsNullOrWhiteSpace(eventId)) | ||
| { | ||
| this.vanishDeletedEventIds.TryAdd(eventId, 0); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| public User SetFromEvent(Event e) | ||
| { | ||
| return this.users.AddOrUpdate( | ||
| e.PublicKey, | ||
| key => new User { PublicKey = key, EventId = e.Id }, | ||
| (key, user) => user with { EventId = e.Id }); | ||
| } | ||
|
|
||
| public User Vanish(string publicKey, DateTimeOffset timestamp) | ||
| public bool IsVanishDeletedEvent(string eventId) | ||
| { | ||
| return this.users.AddOrUpdate( | ||
| publicKey, | ||
| key => new User { PublicKey = key, LastVanished = timestamp }, | ||
| (key, user) => user with { LastVanished = timestamp }); | ||
| return this.vanishDeletedEventIds.ContainsKey(eventId); | ||
| } |
There was a problem hiding this comment.
vanishDeletedEventIds grows monotonically and is never evicted. A user vanishing with a large history (or repeated vanishes across many users) can lead to unbounded memory growth over time. Consider bounding this structure (e.g., time-based expiration via MemoryCache, a size cap + eviction, or persisting/querying a durable marker in the DB instead of keeping all ids indefinitely in-memory).
| /// Enables non-standard AND-tag filters using the '&' modifier (e.g. "&p": ["a","b"]). | ||
| /// When disabled, any '&x' filter keys are rejected as unsupported. | ||
| /// </summary> | ||
| public bool AllowAndTagFilters { get; init; } = false; |
There was a problem hiding this comment.
Defaulting AllowAndTagFilters to false is a behavioral change that can silently disable AND-tag filters unless every deployment explicitly sets Filters:AllowAndTagFilters. Given the relay advertises NIP-119 support and the test factory defaults this to true, consider either (a) defaulting this option to true, or (b) adding "Filters": { "AllowAndTagFilters": true } to appsettings.json to preserve current behavior by default.
Suggested change
| public bool AllowAndTagFilters { get; init; } = false; | |
| public bool AllowAndTagFilters { get; init; } = true; |
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
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.