Scope each publisher's cookies to its own path

CHANGELOG.md

@@ -158,6 +158,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - `[robots-txt]` serves a publisher's `/robots.txt` from the settings. `modules` names what makes the file, in order. `refuse_all` and `allow_all` are Trusted Server's own, and any other name is a module that contributes rules through the new `RobotsTxtContributor` extension point, declared with `with_robots_txt_contributor`. Trusted Server writes the file, so a contributor cannot produce a malformed one, a refusal cannot be loosened, and a path in `always_allow` stays open to every crawler refused the whole site. With `refuse_all` selected every response an adapter finalizes carries `X-Robots-Tag: noindex, nofollow`. A module is asked for the host the request was addressed to, never one a forwarded header names, and is given nothing of the crawler's connection. Its answer is held for that host by the loading cache until its refresh is due. A module that cannot answer has its last held answer served, and with none held, or with no rule in the whole file, the answer is `503` and never an empty file. On Fastly the file is answered before the Edge Cookie steps and the request filters. Without the section `/robots.txt` reaches the publisher's origin as before.
 - `ts keys rotate` and `ts keys deactivate` rotate and retire the request signing keys from an operator's machine. They write the two stores the service links as `jwks_store` and `signing_keys` through the `fastly` CLI, with the token from `FASTLY_API_TOKEN`. A private key reaches `fastly` on its standard input and never in its arguments, and a key id no platform can store is refused before anything is written. A command that cannot read the config store writes nothing, and a delete that failed part way can be run again.
 - A deployment may address a request by its path before its configuration is chosen, so one host serves several publishers, each under a path of its own and from its own app-config blob. It supplies a `RequestAddressing` from `trusted_server_core::addressing`, through `trusted_server_adapter_fastly::run_with_addressing` or `TrustedServerApp::routes_with_addressing` on the other adapters. The hook answers either that a request is addressed by its host, as before, or the selector values that take the place of `{host}`, the path the request continues with and the prefix it removed. The first value whose blob exists is read and a miss on every value is answered `421`, never from a key the hook did not name. The request is routed at the continuing path with its query kept, and carries the prefix as a `RemovedPathPrefix` extension, which a deployment's own middleware puts back into its pages. Adapters that read one blob ignore the values. With no hook every request is served as before.
+- On a request addressed by path, core writes the cookies it owns, being the Edge Cookie, its resolved marker, the pull-sync marker and the tester cookie, with the removed prefix as their `Path`, so the browser sends them only on requests under that publisher's path. A request with no removed prefix, an empty one, `/` or one that cannot be a cookie's `Path` writes `Path=/` as before. Where core writes under the prefix, the Edge Cookie and the pull-sync marker are read from the first cookie of each name, which is the one under the publisher's path in a browser that lists longer paths first, as RFC 6265 recommends, and elsewhere from the last, as before. A withdrawal on an addressed request expires the Edge Cookie, its resolved marker and the pull-sync marker under the prefix and under `/`, and tombstones the row of each Edge Cookie the request carried. A cookie under `/` is shared by every publisher on the host that uses its name, and an Edge Cookie a visitor already holds under `/` stays under `/` until it expires or is withdrawn. Cookies an origin, a module, a consent tool or the page writes under `/` are still sent on every publisher's requests and forwarded to every origin that receives the `Cookie` header.
 - One Fastly service can serve several publishers. A `TS__CONFIG__KEY` entry in the `trusted_server_config` store that contains `{host}` names an app-config blob for each host, pushed with `ts config push --key <host>`, and each request reads the blob under its own host. Each host's blob names its permission policy file and pins its SHA-256, and the push keeps each file `--permissions` supplies once, in a shared entry under `permissions_<sha256>`, written before the blob, which a live push writes a minute after it writes an entry. A host with no blob is answered `421 Misdirected Request`, never from the logical store ID or from another host's blob, a malformed selector is answered `500`, and a sandbox that serves several requests keeps an application for each blob, found only by that blob's key. Without the entry, or with a selector that has no placeholder, the service reads one blob. `scripts/config-by-host-local-test.sh` runs it under Viceroy.
 - `ts audit generate` writes the `[[fetch]]` and `[[serve]]` entries that run the page changes of the modules it selects on every HTML page, and `trusted-server.example.toml` documents the entries with every middleware the stock modules supply. Core lists those middleware with `supplied_middleware(builders)`.
 - Page changes as middleware. A module registers a change it makes to a page with `.with_middleware(...)`, and the ordered `[[fetch]]` and `[[serve]]` entries of the settings say which pages each one runs on and in what order. A fetch middleware runs on the page as the origin sent it and what it writes is stored with a shared template. A serve middleware runs on each reader's copy, whether the page came from the store or from the origin, and nothing it writes is stored. Where text a middleware writes into a script would end the script, or begin `<script` after `<!--`, its `<` is written as `\u003c`. An entry with no path after one with `path = "/"` is refused, because both cover every page.

crates/trusted-server-adapter-fastly/src/app.rs

@@ -1044,10 +1044,10 @@ async fn run_named_route(
             }
         }
         NamedRouteHandler::SetTester => {
-            handle_set_tester(&state.settings, services.public_prefix())
+            handle_set_tester(&state.settings, &services.public_prefix().for_request(&req))
         }
         NamedRouteHandler::ClearTester => {
-            handle_clear_tester(&state.settings, services.public_prefix())
+            handle_clear_tester(&state.settings, &services.public_prefix().for_request(&req))
         }
         NamedRouteHandler::Permissions | NamedRouteHandler::Config | NamedRouteHandler::Data => {
             unreachable!("the inspection endpoints are answered before EC setup")
@@ -2606,6 +2606,34 @@ mod tests {
         );
     }
 
+    /// On a request addressed by path, the tester cookie is written under
+    /// the removed prefix, as every other cookie core writes is.
+    #[test]
+    fn dispatch_set_tester_writes_under_the_removed_prefix() {
+        let mut settings = test_settings();
+        settings.tester_cookie.enabled = true;
+        let state = app_state_for_settings(settings);
+        let router = TrustedServerApp::routes_for_state(&state);
+        let mut request = empty_request(Method::GET, "/_ts/set-tester");
+        request
+            .extensions_mut()
+            .insert(trusted_server_core::addressing::RemovedPathPrefix::new(
+                "/publisher-a.example".to_owned(),
+            ));
+        let response = route(&router, request);
+
+        let set_cookie = response
+            .headers()
+            .get(header::SET_COOKIE)
+            .expect("should set tester cookie")
+            .to_str()
+            .expect("should render set-cookie as utf-8");
+        assert!(
+            set_cookie.contains("; Path=/publisher-a.example; "),
+            "tester cookie should be under the removed prefix: {set_cookie}"
+        );
+    }
+
     #[test]
     fn dispatch_permissions_answers_as_data_and_as_a_page_and_writes_nothing() {
         let router = test_router();

crates/trusted-server-core/src/addressing.rs

@@ -10,8 +10,10 @@
 //!
 //! The removed prefix travels on the request as a [`RemovedPathPrefix`]
 //! extension, where a module's request preparer or a handler reads it. Core
-//! never writes it into a response. A deployment whose pages must carry the
-//! prefix puts it back with a middleware of its own.
+//! writes it into a response only as the `Path` of the cookies core writes,
+//! see [`PublicPrefix::for_request`](crate::prefix::PublicPrefix::for_request).
+//! A deployment whose pages must carry the prefix, in their links for
+//! example, puts it back with a middleware of its own.
 
 use std::sync::Arc;
 
@@ -93,7 +95,9 @@ impl RequestAddress {
     /// An address that names `selector_values` in order, continues the
     /// request at `path` and records `removed_prefix` as the prefix removed.
     ///
-    /// The request's query is kept, so `path` carries none.
+    /// The request's query is kept, so `path` carries none. Any
+    /// `removed_prefix` is accepted, and where it cannot be a cookie's
+    /// `Path`, core writes the request's cookies under `/`.
     ///
     /// # Errors
     ///
@@ -172,9 +176,10 @@ impl RequestAddressing for ByHostAddressing {
 /// The path prefix a [`RequestAddressing`] removed from a request, carried
 /// as a request extension.
 ///
-/// Present only on a request a hook addressed. Putting the prefix back into
-/// a response, such as into the links of a page, is the deployment's job,
-/// done by a middleware of its own.
+/// Present only on a request a hook addressed. Core writes its own cookies
+/// under the prefix. Putting the prefix back into the rest of a response,
+/// such as into the links of a page, is the deployment's job, done by a
+/// middleware of its own.
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub struct RemovedPathPrefix(String);
 
@@ -276,6 +281,30 @@ fn addressed_handler(
     }
 }
 
+/// Whether `path` can be the `Path` attribute of a cookie.
+///
+/// It starts with `/` and holds only visible ASCII other than `;`, so
+/// written into a `Set-Cookie` header it cannot end the attribute and add
+/// another.
+#[must_use]
+pub fn is_cookie_path(path: &str) -> bool {
+    path.starts_with('/')
+        && path
+            .bytes()
+            .all(|byte| byte.is_ascii_graphic() && byte != b';')
+}
+
+/// The `Path` core writes its cookies under on a request whose removed
+/// prefix is `prefix`, or `None` for `/`.
+///
+/// An empty prefix, `/` itself and a prefix that cannot be a cookie's `Path`
+/// all give `None`, so such a request writes and reads its cookies as a
+/// request with no removed prefix does.
+#[must_use]
+pub(crate) fn cookie_path_for(prefix: &str) -> Option<&str> {
+    (prefix != "/" && is_cookie_path(prefix)).then_some(prefix)
+}
+
 /// Whether `path` is one a request can continue with, which is a
 /// percent-encoded ASCII path as a request carries it, so every adapter
 /// routes the same path.
@@ -414,6 +443,50 @@ mod tests {
         }
     }
 
+    /// The hook in this module's documentation, which addresses a request
+    /// by host when [`RequestAddress::new`] refuses it.
+    struct DocumentedHook;
+
+    impl RequestAddressing for DocumentedHook {
+        fn address(&self, _host: Option<&str>, path: &str) -> Addressing {
+            let rest = path.strip_prefix('/').unwrap_or(path);
+            let (segment, tail) = rest.split_once('/').unwrap_or((rest, ""));
+            RequestAddress::new(
+                vec![segment.to_owned()],
+                format!("/{tail}"),
+                format!("/{segment}"),
+            )
+            .map_or(Addressing::ByHost, Addressing::Addressed)
+        }
+    }
+
+    /// A removed prefix that cannot be a cookie's `Path` still addresses the
+    /// request, so an adapter that chooses a blob by the selector values
+    /// answers it as it answers any value with no blob. Core's cookies for
+    /// that request go under `/`.
+    #[test]
+    fn a_prefix_that_is_not_a_cookie_path_still_addresses_the_request() {
+        let Addressing::Addressed(address) =
+            DocumentedHook.address(Some("front.example"), "/a;b/news")
+        else {
+            panic!("should address `/a;b/news` by its first segment");
+        };
+        assert_eq!(address.selector_values(), ["a;b"]);
+        assert_eq!(address.path(), "/news");
+        assert_eq!(address.removed_prefix(), "/a;b");
+
+        let mut addressed = request("https://front.example/a;b/news", Some("front.example"));
+        address_request(&DocumentedHook, &mut addressed).expect("should address the request");
+        assert_eq!(addressed.uri().path(), "/news");
+        assert_eq!(
+            crate::prefix::PublicPrefix::default()
+                .for_request(&addressed)
+                .cookie_path(),
+            "/",
+            "a prefix holding `;` should never reach a cookie's `Path`"
+        );
+    }
+
     #[test]
     fn the_error_carries_nothing_from_the_request() {
         let error = RequestAddress::new(Vec::new(), "/private?id=reader".to_owned(), String::new())

crates/trusted-server-core/src/cookies.rs

@@ -29,10 +29,33 @@ pub fn parse_cookies_to_jar(s: &str) -> CookieJar {
     jar
 }
 
+/// Parses a cookie string into a [`CookieJar`] that keeps the first cookie
+/// of each name.
+///
+/// RFC 6265 section 5.4 says a browser SHOULD list the cookie with the
+/// longer `Path` first. In a browser that does, on a request under a
+/// publisher's path this keeps the publisher's cookie over one of the same
+/// name written under `/`. A browser that does not may list the root cookie
+/// first, and the root cookie is then the one kept.
+fn parse_cookies_to_jar_keeping_first(s: &str) -> CookieJar {
+    let mut jar = CookieJar::new();
+    for cookie in Cookie::split_parse(s.trim().to_owned()).filter_map(Result::ok) {
+        if jar.get(cookie.name()).is_none() {
+            jar.add_original(cookie);
+        }
+    }
+    jar
+}
+
 /// Extracts and parses cookies from an HTTP request.
 ///
 /// Attempts to parse the Cookie header into a [`CookieJar`] for easy access
-/// to individual cookies.
+/// to individual cookies. Where core writes its cookies under the prefix
+/// request addressing removed, the first cookie of each name is kept, which
+/// is the one under the publisher's path in a browser that lists longer
+/// paths first. Otherwise the last is kept, as [`parse_cookies_to_jar`]
+/// keeps it, which covers a request with no removed prefix and one whose
+/// removed prefix is empty, `/` or not a cookie path.
 ///
 /// # Errors
 ///
@@ -47,14 +70,46 @@ pub fn handle_request_cookies(
                 .change_context(TrustedServerError::InvalidHeaderValue {
                     message: "Cookie header contains invalid UTF-8".to_owned(),
                 })?;
-        let jar = parse_cookies_to_jar(header_value_str);
+        let under_removed_prefix = req
+            .extensions()
+            .get::<crate::addressing::RemovedPathPrefix>()
+            .and_then(|prefix| crate::addressing::cookie_path_for(prefix.as_str()))
+            .is_some();
+        let jar = if under_removed_prefix {
+            parse_cookies_to_jar_keeping_first(header_value_str)
+        } else {
+            parse_cookies_to_jar(header_value_str)
+        };
         Ok(Some(jar))
     } else {
         log::debug!("No cookie header found in request");
         Ok(None)
     }
 }
 
+/// Every value of the cookie `name` in the request's `Cookie` header, in the
+/// order the browser sent them, parsed as [`handle_request_cookies`] parses
+/// them.
+///
+/// A browser sends one pair for each cookie of a name it holds, such as one
+/// under `/` and one under a publisher's path, where a [`CookieJar`] keeps
+/// one. A missing or non-UTF-8 header gives none.
+#[must_use]
+pub(crate) fn cookie_values<B>(req: &Request<B>, name: &str) -> Vec<String> {
+    let Some(header_value) = req
+        .headers()
+        .get(header::COOKIE)
+        .and_then(|value| value.to_str().ok())
+    else {
+        return Vec::new();
+    };
+    Cookie::split_parse(header_value.trim())
+        .filter_map(Result::ok)
+        .filter(|cookie| cookie.name() == name)
+        .map(|cookie| cookie.value().to_owned())
+        .collect()
+}
+
 /// Returns the named value from the request's selected `Cookie` header.
 ///
 /// Values are trimmed and may contain additional `=` characters. A missing or
@@ -128,6 +183,62 @@ mod tests {
         assert_eq!(jar.get("c1").expect("should have cookie c1").value(), "v2");
     }
 
+    /// A browser that lists the cookie with the longer path first sends the
+    /// publisher's own cookie first, so on an addressed request that cookie
+    /// is the one read.
+    #[test]
+    fn an_addressed_request_reads_the_first_cookie_of_a_name() {
+        let mut req = build_request(Some("ts-ec=publisher; ts-ec=root; other=1"));
+        req.extensions_mut()
+            .insert(crate::addressing::RemovedPathPrefix::new(
+                "/publisher-a.example".to_owned(),
+            ));
+        let jar = handle_request_cookies(&req)
+            .expect("should parse cookies")
+            .expect("should have cookie jar");
+
+        assert_eq!(jar.get("ts-ec").map(Cookie::value), Some("publisher"));
+        assert_eq!(jar.get("other").map(Cookie::value), Some("1"));
+    }
+
+    #[test]
+    fn a_request_with_no_removed_prefix_reads_the_last_cookie_of_a_name() {
+        let req = build_request(Some("ts-ec=publisher; ts-ec=root"));
+        let jar = handle_request_cookies(&req)
+            .expect("should parse cookies")
+            .expect("should have cookie jar");
+
+        assert_eq!(jar.get("ts-ec").map(Cookie::value), Some("root"));
+    }
+
+    /// An empty removed prefix, `/`, or a prefix that cannot be a cookie's
+    /// `Path` puts core's cookies under `/`, so the request reads them as a
+    /// request with no removed prefix does.
+    #[test]
+    fn a_removed_prefix_whose_cookies_go_under_the_root_reads_the_last_cookie_of_a_name() {
+        for prefix in ["", "/", "/a;b"] {
+            let mut req = build_request(Some("ts-ec=publisher; ts-ec=root"));
+            req.extensions_mut()
+                .insert(crate::addressing::RemovedPathPrefix::new(prefix.to_owned()));
+            let jar = handle_request_cookies(&req)
+                .expect("should parse cookies")
+                .expect("should have cookie jar");
+
+            assert_eq!(
+                jar.get("ts-ec").map(Cookie::value),
+                Some("root"),
+                "removed prefix `{prefix}`"
+            );
+        }
+    }
+
+    #[test]
+    fn cookie_values_gives_every_value_of_a_name_in_the_order_sent() {
+        let req = build_request(Some("ts-ec=publisher; other=1; ts-ec=root"));
+        assert_eq!(cookie_values(&req, "ts-ec"), ["publisher", "root"]);
+        assert!(cookie_values(&build_request(None), "ts-ec").is_empty());
+    }
+
     #[test]
     fn test_parse_cookies_to_jar_empty() {
         let cookie_str = "";