Repository navigation
Share only the evidence usage sharing accepts unless asked for all - #35
Merged
Merged
Conversation
Usage sharing should share what the specification's accepted evidence section allows unless the caller asks to share everything: headers other than Cookie, cookies, session values and query string parameters starting 51D_, the parameters the caller names, and any other evidence. The email address, the salt for hashed email identifiers, the query string and the path are never shared. Naming the session cookie keeps it in evidence but never sends it, and a repeat that differs only in its session id and sequence is not shared again. These tests fail on rc, where every key is shared by default and the session cookie is never kept.
The usage sharing filter shared every piece of evidence whenever no cookie, parameter list or header list was given, which is how the on-premise builder adds it, so every cookie, form field and query string parameter was sent. Sharing everything is now only done when shareAll is true, which defaults to false, as the specification's share all evidence option does. The filter now also - splits a key at its first dot, so a name containing a dot is matched whole, - never shares id.email, id.salt, request-query or request-path, in any mode, as .NET does, - does not share the Cookie header, which carries every cookie, unless all evidence is shared, - matches the header and parameter lists without regard to case, - takes includeSession, which it read but nothing set, so the session cookie named by the cookie option never reached the tracker. ShareUsage builds three filters, as .NET does. The pipeline keeps the named session cookie, the data sent never includes it, and the tracker leaves out the session id and sequence number, which made every request look new. The session id and sequence number stay in evidence because the usage data carries them separately.
This was referenced Nov 6, 2026
Merged
.NET keeps two settings apart. SetTrackSession, false by default, decides whether the session cookie is used to tell sessions apart, and SetAspSessionCookieName only names the cookie, with no effect unless tracking is on. Here naming the cookie turned tracking on. The session cookie test now turns tracking on with trackSession. Two new tests check that the cookie alone has no effect, and that with tracking on a repeat is shared again for a new session but not within the same session.
ShareUsage takes trackSession, false by default, as .NET's SetTrackSession is. The pipeline's filter and the tracker's filter include the session cookie named by cookie only when it is true, so the cookie has no effect otherwise, as .NET's SetAspSessionCookieName has none. The data sent still leaves the session cookie out unless shareAll is true. The type declarations are updated to match, and the declaration for ShareUsageTrackerEvidenceKeyFilter is now the one tsc writes. The usage sharing example's configuration, which lists every option, now lists trackSession and shareAll with their defaults.
jwrosewell
marked this pull request as ready for review
November 7, 2026 02:13
This was referenced Nov 7, 2026
Draft
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.
This is the fix for #34, which can close once it reaches main. Session tracking follows .NET, which is the model for usage sharing.
Problem
ShareUsageEvidenceKeyFilterswitched on sharing everything whenever no cookie and no lists were given.this.includeSessionand nothing set it, sofilterEvidenceKey('cookie.sessionid')gaveundefined. The session cookie named by thecookieoption was never kept for the tracker, andShareUsagestored the option astrackingCookieand never read it again.cookie, where .NET has two settings.SetTrackSession, false by default (Constants.cs lines 88 to 95), decides whether the session cookie is used to tell sessions apart, andSetAspSessionCookieNameonly names the cookie and "has no effect if TrackSession is false". The first version of this pull request made namingcookieturn tracking on.Change
The filter follows the accepted evidence section and the configuration options of the usage sharing specification, and the .NET filter.
ShareUsagetakesshareAll, false by default, as the specification's "Share all Evidence" is. OnlyshareAll: trueshares everything.51d_, query string and form parameters only when they start51d_or are onqueryWhitelist, and any other evidence.queryWhitelist: nullstill shares every parameter.id.email,id.salt,request-queryandrequest-pathare never shared, in any mode, matched without regard to case as the whole key or after a., as in .NET'sEvidenceKeyFilterShareUsage.., soquery.valid.emailis checked asvalid.email.includeSession.ShareUsagebuilds three filters, as .NET's ShareUsageBase.cs lines 636 to 666 does. The filter for the data sent leaves the session cookie out, and the newShareUsageTrackerEvidenceKeyFilterleaves outquery.session-idandquery.sequence, which differ on every request and made every request look new to the tracker. The session id and sequence are added to the parameters kept, as .NET does, because the usage data carries them separately.ShareUsagetakestrackSession, false by default, as .NET'sSetTrackSessionis. Only when it is true do the pipeline's filter and the tracker's filter include the session cookie named bycookie. A repeat from a new session is then shared again, while a repeat within the same session is not.cookieonly names the session cookie, and has no effect unlesstrackSessionis true. The session cookie is not sent unlessshareAllis true.types/are updated to match.shareUsage.d.tsandshareUsageTrackerEvidenceKeyFilter.d.tsare now exactly whattscwrites from the sources.trackSessionandshareAllwith their defaults.For review
shareAllis set. The specification said the default block list "SHOULD include thecookiesHTTP header". HTTP has no header calledCookies, and the request header that carries cookies isCookie(RFC 6265 section 5.4).Evidence.addFromRequest()adds every request header, soheader.cookiecarries every cookie, and without this the session and sign-in cookies the cookie rule leaves out would still be sent. The51d_cookies are still shared, as their owncookie.entries. specifications#89, now merged into rc, put theCookieheader in the specification, and .NET now blocks it too (51degrees-dotnet#81).cookiekeeps its default, which is no name. .NET's default,asp.net_sessionid, is the ASP.NET session cookie, and Java's,JSESSIONID, is the servlet one. Node has no common session cookie name. For example, express-session usesconnect.sidunless told otherwise. So no name would be right by default, andtrackSession: trueneedscookieset to have any effect, as the JSDoc says.Evidence.add()keeps it only when tracking is on, so thatcookiehas no effect at all otherwise. What is sent is the same.shareAlloverrides session tracking, as in .NET. WithshareAll: true, every cookie is shared and compared, the session cookie included, whatevertrackSessionsays. .NET's share all filters do the same (ShareUsageBase.cs lines 659 to 666).trackingCookieis still set fromcookieand still never read. It is left in place because it is part of the declared type.Tests
tests/shareUsageEvidenceKeyFilter.test.jsis new. It ports the .NET filter tests and adds the cases the issue lists:query.id.email,header.id.email,cookie.a.b,session.org.example.attribute,query.valid.email, the session cookie, and lists in mixed case.tests/shareUsage.test.jssends through the local receiver and checks what reaches it: the default settings, the never-shared keys,shareAll, a parameter name with a dot, and a repeat that differs only in its session id and sequence.tests/shareUsage.test.jscover session tracking.share usage - session cookieturns tracking on and checks the session cookie is kept but not sent.share usage - session cookie without session trackingnames the cookie without turning tracking on, and checks that tracking is off by default, the cookie is not kept, and the same evidence from a second session is not sent again.share usage - session trackingturns tracking on and checks that the same evidence is sent again for a new session but not for a repeat in the same session, and that the cookie is never sent.Checks
filterEvidenceKey('cookie.sessionid')givingundefined, the session cookie and email address reaching the receiver, and 2 records where 1 was expected for the repeat. All 14 existing tests passed, and ESLint passed (run 42195395204, first attempt).share usage - session cookie without session trackingreceived 2 records where 1 was expected, because naming the cookie turned tracking on. The other two session tests passed, because they pin what tracking does when it is on, which naming the cookie already did. The other 30 tests, ESLint and the usage sharing example passed (run 42320919381).tscwrites (run 42321441878).Found, not changed
Release note
Usage sharing now shares only the evidence the specification accepts unless
shareAll: trueis given, and never shares an email address, its salt, the query string or the path. A newtrackSessionoption, false by default, uses the session cookie named bycookieto tell sessions apart, as .NET'sSetTrackSessiondoes.