diff --git a/.github/dependabot.yml b/.github/dependabot.yml index 492d67d0..7d2c7033 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -4,6 +4,10 @@ updates: directory: / schedule: interval: monthly + groups: + github-actions: + patterns: + - "*" - package-ecosystem: npm directory: / @@ -12,3 +16,7 @@ updates: ignore: - dependency-name: "*" update-types: ["version-update:semver-major"] + groups: + npm-dependencies: + patterns: + - "*" diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 13336155..004c35ed 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -37,6 +37,10 @@ jobs: - Node.js 19.x - Node.js 20.x - Node.js 21.x + - Node.js 22.x + - Node.js 23.x + - Node.js 24.x + - Node.js 25.x include: - name: Node.js 0.8 @@ -124,13 +128,25 @@ jobs: node-version: "19.9" - name: Node.js 20.x - node-version: "20.11" + node-version: "20" - name: Node.js 21.x - node-version: "21.6" + node-version: "21" + + - name: Node.js 22.x + node-version: "22" + + - name: Node.js 23.x + node-version: "23" + + - name: Node.js 24.x + node-version: "24" + + - name: Node.js 25.x + node-version: "25" steps: - - uses: actions/checkout@1af3b93b6815bc44a9784bd300feb67ff0d1eeb3 # v6.0.0 + - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 - name: Install Node.js ${{ matrix.node-version }} shell: bash -eo pipefail -l {0} @@ -139,7 +155,12 @@ jobs: if [[ "${{ matrix.node-version }}" == 0.* && "$(cut -d. -f2 <<< "${{ matrix.node-version }}")" -lt 10 ]]; then nvm install --alias=npm 0.10 nvm use ${{ matrix.node-version }} - sed -i '1s;^.*$;'"$(printf '#!%q' "$(nvm which npm)")"';' "$(readlink -f "$(which npm)")" + if [[ "$(npm -v)" == 1.1.* ]]; then + nvm exec npm npm install -g npm@1.1 + ln -fs "$(which npm)" "$(dirname "$(nvm which npm)")/npm" + else + sed -i '1s;^.*$;'"$(printf '#!%q' "$(nvm which npm)")"';' "$(readlink -f "$(which npm)")" + fi npm config set strict-ssl false fi dirname "$(nvm which ${{ matrix.node-version }})" >> "$GITHUB_PATH" @@ -208,7 +229,7 @@ jobs: fi - name: Upload code coverage - uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5.0.0 + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 if: steps.list_env.outputs.nyc != '' with: name: coverage-${{ matrix.node-version }} @@ -222,14 +243,14 @@ jobs: needs: test runs-on: ubuntu-latest steps: - - uses: actions/checkout@1af3b93b6815bc44a9784bd300feb67ff0d1eeb3 # v6.0.0 + - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 - name: Install lcov shell: bash run: sudo apt-get -y install lcov - name: Collect coverage reports - uses: actions/download-artifact@018cc2cf5baa6db3ef3c5f8a56943fffe632ef53 # v6.0.0 + uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 with: path: ./coverage diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index 8534ee51..139ec706 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -34,11 +34,11 @@ jobs: steps: - name: Checkout repository - uses: actions/checkout@1af3b93b6815bc44a9784bd300feb67ff0d1eeb3 # v6.0.0 + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 # Initializes the CodeQL tools for scanning. - name: Initialize CodeQL - uses: github/codeql-action/init@fe4161a26a8629af62121b670040955b330f9af2 # v4.31.6 + uses: github/codeql-action/init@54f647b7e1bb85c95cddabcd46b0c578ec92bc1a # v4.36.3 with: languages: javascript # If you wish to specify custom queries, you can do so here or in a config file. @@ -61,6 +61,6 @@ jobs: # ./location_of_script_within_repo/buildscript.sh - name: Perform CodeQL Analysis - uses: github/codeql-action/analyze@fe4161a26a8629af62121b670040955b330f9af2 # v4.31.6 + uses: github/codeql-action/analyze@54f647b7e1bb85c95cddabcd46b0c578ec92bc1a # v4.36.3 with: category: "/language:javascript" \ No newline at end of file diff --git a/.github/workflows/scorecard.yml b/.github/workflows/scorecard.yml index 169262af..2bb15b23 100644 --- a/.github/workflows/scorecard.yml +++ b/.github/workflows/scorecard.yml @@ -33,7 +33,7 @@ jobs: steps: - name: "Checkout code" - uses: actions/checkout@1af3b93b6815bc44a9784bd300feb67ff0d1eeb3 # v6.0.0 + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 with: persist-credentials: false @@ -60,7 +60,7 @@ jobs: # Upload the results as artifacts (optional). Commenting out will disable uploads of run results in SARIF # format to the repository Actions tab. - name: "Upload artifact" - uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5.0.0 + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 with: name: SARIF file path: results.sarif @@ -68,6 +68,6 @@ jobs: # Upload the results to GitHub's code scanning dashboard. - name: "Upload to code-scanning" - uses: github/codeql-action/upload-sarif@fe4161a26a8629af62121b670040955b330f9af2 # v4.31.6 + uses: github/codeql-action/upload-sarif@54f647b7e1bb85c95cddabcd46b0c578ec92bc1a # v4.36.3 with: sarif_file: results.sarif \ No newline at end of file diff --git a/HISTORY.md b/HISTORY.md index fae1a0f1..251d5fa7 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -1,3 +1,9 @@ +unreleased +========== + + * Enforce RFC 6265 section 5.1.4 path matching for the session cookie path + * Fix `secure: 'auto'` and `sameSite: 'auto'` not being resolved when `cookie` is a function + 1.19.0 / 2026-01-22 ========== diff --git a/README.md b/README.md index 292b2024..162ea14d 100644 --- a/README.md +++ b/README.md @@ -126,6 +126,12 @@ More information about can be found in [the proposal](https://github.com/privacy Specifies the value for the `Path` `Set-Cookie`. By default, this is set to `'/'`, which is the root path of the domain. +Since 1.19.1, path matching follows [RFC 6265 section 5.1.4][rfc-6265-5.1.4]. This means +the session middleware will only activate when the request path is an exact match or falls +under a segment boundary of the cookie path. For example, a cookie path of `/admin` will +match `/admin` and `/admin/users` but will **not** match `/administrator`. Prior versions +used a simple prefix check that did not enforce segment boundaries. + ##### cookie.priority Specifies the `string` to be the value for the [`Priority` `Set-Cookie` attribute][rfc-west-cookie-priority-00-4.1]. @@ -733,7 +739,7 @@ and other multi-core embedded devices). [connect-session-knex-image]: https://badgen.net/github/stars/llambda/connect-session-knex?label=%E2%98%85 [![★][connect-session-sequelize-image] connect-session-sequelize][connect-session-sequelize-url] A session store using -[Sequelize.js](http://sequelizejs.com/), which is a Node.js / io.js ORM for PostgreSQL, MySQL, SQLite and MSSQL. +[Sequelize.js](https://sequelize.org/), which is a Node.js / io.js ORM for PostgreSQL, MySQL, SQLite and MSSQL. [connect-session-sequelize-url]: https://www.npmjs.com/package/connect-session-sequelize [connect-session-sequelize-image]: https://badgen.net/github/stars/mweibel/connect-session-sequelize?label=%E2%98%85 @@ -758,11 +764,6 @@ and other multi-core embedded devices). [dynamodb-store-url]: https://www.npmjs.com/package/dynamodb-store [dynamodb-store-image]: https://badgen.net/github/stars/rafaelrpinto/dynamodb-store?label=%E2%98%85 -[![★][dynamodb-store-v3-image] dynamodb-store-v3][dynamodb-store-v3-url] Implementation of a session store using DynamoDB backed by the [AWS SDK for JavaScript v3](https://github.com/aws/aws-sdk-js-v3). - -[dynamodb-store-v3-url]: https://www.npmjs.com/package/dynamodb-store-v3 -[dynamodb-store-v3-image]: https://badgen.net/github/stars/FryDay/dynamodb-store-v3?label=%E2%98%85 - [![★][express-etcd-image] express-etcd][express-etcd-url] An [etcd](https://github.com/stianeikeland/node-etcd) based session store. [express-etcd-url]: https://www.npmjs.com/package/express-etcd @@ -868,7 +869,7 @@ based session store. Supports all backends supported by Fortune (MongoDB, Redis, [restsession-url]: https://www.npmjs.com/package/restsession [restsession-image]: https://badgen.net/github/stars/jankal/restsession?label=%E2%98%85 -[![★][sequelstore-connect-image] sequelstore-connect][sequelstore-connect-url] A session store using [Sequelize.js](http://sequelizejs.com/). +[![★][sequelstore-connect-image] sequelstore-connect][sequelstore-connect-url] A session store using [Sequelize.js](https://sequelize.org/). [sequelstore-connect-url]: https://www.npmjs.com/package/sequelstore-connect [sequelstore-connect-image]: https://badgen.net/github/stars/MattMcFarland/sequelstore-connect?label=%E2%98%85 @@ -883,11 +884,6 @@ based session store. Supports all backends supported by Fortune (MongoDB, Redis, [session-pouchdb-store-url]: https://www.npmjs.com/package/session-pouchdb-store [session-pouchdb-store-image]: https://badgen.net/github/stars/solzimer/session-pouchdb-store?label=%E2%98%85 -[![★][@cyclic.sh/session-store-image] @cyclic.sh/session-store][@cyclic.sh/session-store-url] A DynamoDB-based session store for [Cyclic.sh](https://www.cyclic.sh/) apps. - -[@cyclic.sh/session-store-url]: https://www.npmjs.com/package/@cyclic.sh/session-store -[@cyclic.sh/session-store-image]: https://badgen.net/github/stars/cyclic-software/session-store?label=%E2%98%85 - [![★][@databunker/session-store-image] @databunker/session-store][@databunker/session-store-url] A [Databunker](https://databunker.org/)-based encrypted session store. [@databunker/session-store-url]: https://www.npmjs.com/package/@databunker/session-store @@ -1048,6 +1044,7 @@ On Windows, use the corresponding command; [MIT](LICENSE) +[rfc-6265-5.1.4]: https://datatracker.ietf.org/doc/html/rfc6265#section-5.1.4 [rfc-6265bis-03-4.1.2.7]: https://tools.ietf.org/html/draft-ietf-httpbis-rfc6265bis-03#section-4.1.2.7 [rfc-cutler-httpbis-partitioned-cookies]: https://tools.ietf.org/html/draft-cutler-httpbis-partitioned-cookies/ [rfc-west-cookie-priority-00-4.1]: https://tools.ietf.org/html/draft-west-cookie-priority-00#section-4.1 diff --git a/index.js b/index.js index 1a509318..3e23e901 100644 --- a/index.js +++ b/index.js @@ -158,15 +158,16 @@ function session(options) { store.generate = function(req){ req.sessionID = generateId(req); req.session = new Session(req); - req.session.cookie = new Cookie(typeof cookieOptions === 'function' ? cookieOptions(req) : cookieOptions); + var resolvedCookieOptions = typeof cookieOptions === 'function' ? cookieOptions(req) : cookieOptions; + req.session.cookie = new Cookie(resolvedCookieOptions); var isSecure = issecure(req, trustProxy); - if (cookieOptions.secure === 'auto') { + if (resolvedCookieOptions.secure === 'auto') { req.session.cookie.secure = isSecure; } - if (cookieOptions.sameSite === 'auto') { + if (resolvedCookieOptions.sameSite === 'auto') { req.session.cookie.sameSite = isSecure ? 'none' : 'lax'; } }; @@ -200,12 +201,15 @@ function session(options) { // pathname mismatch var originalPath = parseUrl.original(req).pathname || '/' var resolvedCookieOptions = typeof cookieOptions === 'function' ? cookieOptions(req) : cookieOptions - if (originalPath.indexOf(resolvedCookieOptions.path || '/') !== 0) { + var cfgPath = resolvedCookieOptions.path || '/' + + if (!rfcPathMatch(originalPath, cfgPath)) { debug('pathname mismatch') next() return } + // ensure a secret is available or bail if (!secret && !req.secret) { next(new Error('secret option required for sessions')); @@ -523,6 +527,42 @@ function session(options) { }; }; +/** + * Check if the cookiePath matches the requestPath following the + * rules in RFC 6265 section 5.1.4. + * + * @param {String} requestPath + * @param {String} cookiePath + * @return {Boolean} + * @private + */ + +function rfcPathMatch(requestPath, cookiePath) { + // Normalize inputs (Node 0.8-safe) + requestPath = (typeof requestPath === 'string' && requestPath.length) ? requestPath : '/'; + cookiePath = (typeof cookiePath === 'string' && cookiePath.length) ? cookiePath : '/'; + + // Root cookie matches everything + if (cookiePath === '/') return true; + + // Exact match + if (requestPath === cookiePath) return true; + + // Prefix match + if (requestPath.indexOf(cookiePath) === 0) { + // If cookiePath ends with '/', any longer requestPath is OK + if (cookiePath.charAt(cookiePath.length - 1) === '/') return true; + + // Otherwise the next char after the prefix must be '/' + var nextChar = requestPath.length > cookiePath.length + ? requestPath.charAt(cookiePath.length) + : ''; + return nextChar === '/'; + } + + return false; +} + /** * Generate a session ID for a new session. * @@ -553,7 +593,7 @@ function getcookie(req, name, secrets) { raw = cookies[name]; if (raw) { - if (raw.substr(0, 2) === 's:') { + if (raw.slice(0, 2) === 's:') { val = unsigncookie(raw.slice(2), secrets); if (val === false) { @@ -580,7 +620,7 @@ function getcookie(req, name, secrets) { raw = req.cookies[name]; if (raw) { - if (raw.substr(0, 2) === 's:') { + if (raw.slice(0, 2) === 's:') { val = unsigncookie(raw.slice(2), secrets); if (val) { @@ -637,7 +677,8 @@ function hash(sess) { function issecure(req, trustProxy) { // socket is https server - if (req.connection && req.connection.encrypted) { + var socket = req.socket || req.connection; + if (socket && socket.encrypted) { return true; } @@ -655,7 +696,7 @@ function issecure(req, trustProxy) { var header = req.headers['x-forwarded-proto'] || ''; var index = header.indexOf(','); var proto = index !== -1 - ? header.substr(0, index).toLowerCase().trim() + ? header.slice(0, index).toLowerCase().trim() : header.toLowerCase().trim() return proto === 'https'; diff --git a/test/session.js b/test/session.js index 46fed763..405e3ddf 100644 --- a/test/session.js +++ b/test/session.js @@ -800,6 +800,31 @@ describe('session()', function(){ .expect(200, 'false', done) }) }) + + describe('when request socket is encrypted', function () { + it('should set secure over TLS', function (done) { + var cert = fs.readFileSync(__dirname + '/fixtures/server.crt', 'ascii') + var server = https.createServer({ + key: fs.readFileSync(__dirname + '/fixtures/server.key', 'ascii'), + cert: cert + }) + + server.on('request', createRequestListener({ secret: 'keyboard cat', cookie: { secure: 'auto' } })) + + var agent = new https.Agent({ ca: cert }) + var createConnection = agent.createConnection + + agent.createConnection = function (options) { + options.servername = 'express-session.local' + return createConnection.call(this, options) + } + + var req = request(server).get('/') + req.agent(agent) + req.expect(shouldSetCookieWithAttribute('connect.sid', 'Secure')) + req.expect(200, done) + }) + }) }) describe('when "cookie" is a function', function () { @@ -853,7 +878,43 @@ describe('session()', function(){ done(); }); }); + + it('should not set cookie when request-path does not match returned path', function (done) { + var cookieCallback = function () { + return { path: '/admin', httpOnly: true, secure: false }; + }; + var server = createServer({ cookie: cookieCallback }); + request(server) + .get('/administrator') + .expect(shouldNotHaveHeader('Set-Cookie')) + .expect(200, done); + }); + + it('should resolve "secure" and "sameSite" set to "auto"', function (done) { + function setup (req) { + req.secure = JSON.parse(req.headers['x-secure']) + } + + function respond (req, res) { + res.end(String(req.secure)) + } + + var cookieCallback = function () { + return { path: '/', secure: 'auto', sameSite: 'auto' }; + }; + var server = createServer(setup, { cookie: cookieCallback }, respond); + request(server) + .get('/') + .set('X-Secure', 'true') + .expect(shouldSetCookieWithAttribute('connect.sid', 'Secure')) + .expect( + shouldSetCookieWithAttributeAndValue('connect.sid', 'SameSite', 'None') + ) + .expect(200, 'true', done); + }); }); + + describe('when "sameSite" set to "auto"', function () { describe('basic functionality', function () { before(function () { @@ -1238,7 +1299,7 @@ describe('session()', function(){ var store = new session.MemoryStore() var server = createServer({ resave: false, store: store }, function (req, res) { if (req.method === 'PUT') { - req.session.token = req.url.substr(1) + req.session.token = req.url.slice(1) } res.end('token=' + (req.session.token || '')) }) @@ -2620,6 +2681,115 @@ describe('session()', function(){ }) }) +describe('path matching (RFC 6265)', function () { + describe('when "path" is "/" (root path)', function () { + before(function () { + this.server = createServer({ cookie: { path: '/' } }) + }) + + it('should set cookie when request-path is "/" (root path)', function (done) { + // RFC 6265 5.1.4: "The cookie-path and the request-path are identical." + request(this.server) + .get('/') + .expect(shouldSetCookie('connect.sid')) + .expect(200, done) + }) + + it('should set cookie when request-path is any path ("/foo")', function (done) { + // RFC 6265 5.1.4: "The cookie-path is a prefix of the request-path, and the last + // character of the cookie-path is %x2F ("/")." + request(this.server) + .get('/foo') + .expect(shouldSetCookie('connect.sid')) + .expect(200, done) + }) + + it('should set cookie when request-path has multiple segments ("/foo/bar/baz")', function (done) { + // RFC 6265 5.1.4: "The cookie-path is a prefix of the request-path, and the last + // character of the cookie-path is %x2F ("/")." + request(this.server) + .get('/foo/bar/baz') + .expect(shouldSetCookie('connect.sid')) + .expect(200, done) + }) + }) + + describe('when "path" is "/admin"', function () { + before(function () { + this.server = createServer({ cookie: { path: '/admin' } }) + }) + + it('should set cookie when request-path and cookie-path are identical ("/admin")', function (done) { + // RFC 6265 5.1.4: "The cookie-path and the request-path are identical." + request(this.server) + .get('/admin') + .expect(shouldSetCookie('connect.sid')) + .expect(200, done) + }) + + it('should set cookie when cookie-path is prefix and last char is "/" ("/admin/")', function (done) { + // RFC 6265 5.1.4: "The cookie-path is a prefix of the request-path, and the last + // character of the cookie-path is %x2F ("/")." + request(this.server) + .get('/admin/') + .expect(shouldSetCookie('connect.sid')) + .expect(200, done) + }) + + it('should set cookie when cookie-path is prefix and next char is "/" ("/admin/users")', function (done) { + // RFC 6265 5.1.4: "The cookie-path is a prefix of the request-path, and the first + // character of the request-path that is not included in the cookie-path is a %x2F ("/") character." + request(this.server) + .get('/admin/users') + .expect(shouldSetCookie('connect.sid')) + .expect(200, done) + }) + + it('should NOT set cookie when cookie-path is not a prefix ("/administrator")', function (done) { + // RFC 6265 5.1.4: None of the path-match conditions are met + request(this.server) + .get('/administrator') + .expect(shouldNotHaveHeader('Set-Cookie')) + .expect(200, done) + }) + }) + + describe('when "path" is "/admin/" (trailing slash)', function () { + before(function () { + this.server = createServer({ cookie: { path: '/admin/' } }) + }) + + it('should set cookie when cookie-path is prefix and last char is "/" ("/admin/x")', function (done) { + // RFC 6265 5.1.4: "The cookie-path is a prefix of the request-path, and the last + // character of the cookie-path is %x2F ("/")." + request(this.server) + .get('/admin/x') + .expect(shouldSetCookie('connect.sid')) + .expect(200, done) + }) + + it('should NOT set cookie when request-path is not prefixed by cookie-path ("/admin")', function (done) { + // RFC 6265 5.1.4: cookie-path "/admin/" is not a prefix of request-path "/admin" + request(this.server) + .get('/admin') + .expect(shouldNotHaveHeader('Set-Cookie')) + .expect(200, done) + }) + + it('should NOT set cookie when cookie-path is not a prefix ("/administrator")', function (done) { + // RFC 6265 5.1.4: None of the path-match conditions are met: + // 1. The paths are not identical + // 2. "/admin/" is not a prefix of "/administrator" + // 3. The prefix condition with next character "/" is not applicable + request(this.server) + .get('/administrator') + .expect(shouldNotHaveHeader('Set-Cookie')) + .expect(200, done) + }) + }) +}) + + function cookie(res) { var setCookie = res.headers['set-cookie']; return (setCookie && setCookie[0]) || undefined;