[JohnMcLear]

A pad connection that sent CLIENT_READY with neither the token cookie nor a legacy token resolved its token to null, and AuthorManager mapped that to the shared token2author:null key. Every such client was the same author, so they could impersonate each other and act as the creator of a pad another token-less client created (delete it, change pad-wide settings).

A connection with no token now gets a fresh token for that session, and checkAccess does the same so HTTP callers (import) can't reach the shared key either. Includes the 3.3.11 CHANGELOG entry.

Tests: new nullAuthorToken.ts backend spec (fails on develop, passes here); full backend mocha 1875 passing, vitest 842 passing, tsc clean.

Reported by @MaBeFHDW.

🤖 Generated with Claude Code

https://claude.ai/code/session_01V4cYEvi9Y4BTPntnkV9tzh

[qodo-code-review]

ⓘ Qodo reviews are paused because the subscription is no longer active. Ask your workspace admin to reactivate the subscription to resume reviews. Manage billing

[qodo-free-for-open-source-projects]

PR Summary by Qodo

Isolate authors for connections without tokens

🐞 Bug fix 🧪 Tests 📝 Documentation 🕐 20-40 Minutes

AI Description

• Give token-less pad connections a session-specific author instead of a shared identity.
• Generate an author token for token-less access checks, including HTTP callers.
• Add regression tests and document the 3.3.11 security fix.

Diagram

graph TD
  Socket["Pad connection"] --> Handler["Pad message handler"] --> Access["Access check"] --> Authors["Author manager"] --> Permissions["Pad permissions"]
  HTTP["HTTP caller"] --> Access

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Enforce non-null tokens in AuthorManager

• ➕ Would protect other callers from creating or reusing the null-token mapping.
• ➖ Does not by itself provide a stable token across messages on one socket connection; callers would still need to retain an identity.

Recommendation: Keep the per-connection fallback in the message handler and the fallback in checkAccess: together they preserve socket identity while covering token-less HTTP calls. A separate non-null guard in AuthorManager could be considered as defense in depth, but it cannot replace session-level token resolution.

Files changed (4) +78 / -1

Bug fix (2) +8 / -1

SecurityManager.tsGenerate a token before resolving token-less access +4/-0

Generate a token before resolving token-less access

• checkAccess now generates an author token when the supplied token is null or undefined, preventing token-less callers from resolving through the shared null-token mapping.

src/node/db/SecurityManager.ts

PadMessageHandler.tsAssign token-less pad connections a session token +4/-1

Assign token-less pad connections a session token

• CLIENT_READY token resolution now generates a token when neither a cookie nor a legacy token is available. The token is retained in the connection's authentication state for subsequent messages.

src/node/handler/PadMessageHandler.ts

Tests (1) +62 / -0

nullAuthorToken.tsTest token-less author and ownership isolation +62/-0

Test token-less author and ownership isolation

• Adds backend tests asserting distinct authors for cookie-less connections, denial of another connection's pad deletion, and distinct authors for null or undefined checkAccess tokens.

src/tests/backend/specs/nullAuthorToken.ts

Documentation (1) +8 / -0

CHANGELOG.mdDocument the 3.3.11 author-isolation security fix +8/-0

Document the 3.3.11 author-isolation security fix

• Adds a security release entry describing the shared-author vulnerability, its impact on pad ownership, and the token-less caller fix.

CHANGELOG.md

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Exports create permanent authors on denial ✓ Resolved

Description

checkAccess() generates a fresh token and maps it to a persistent author before checking whether a
group pad is private or can be created. A token-less export request for an inaccessible group pad
reaches that mapping and leaves author records behind even though access is denied; repeated
requests generate new records because the export route does not issue a token cookie.

Code

src/node/db/SecurityManager.ts[128]

+  if (token == null) token = padutils.generateAuthorToken();

Evidence

The new token reaches the persistent author mapper before the group-pad denial checks. Export calls
this path without issuing a replacement token cookie, so separate token-less requests do not reuse
the mapping.

src/node/db/SecurityManager.ts[126-154]
src/node/padaccess.ts[5-21]
src/node/hooks/express/importexport.ts[38-61]
src/node/db/AuthorManager.ts[112-128]
src/node/db/AuthorManager.ts[194-207]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Token-less export requests can create persistent authors even when group-pad access is denied. Each such HTTP request gets a new token, so repeated requests grow the author database.
## Fix Focus Areas
- src/node/db/SecurityManager.ts[126-154]
- src/node/hooks/express/importexport.ts[38-61]
- src/node/db/AuthorManager.ts[112-128]
## Recommended Fix
Perform all pad-access denial checks before allocating an author. Avoid allocating an author for read-only HTTP access checks that do not use an author ID, while preserving a distinct author for authorized token-less imports and socket sessions.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Token-less requests bypass plugin handling ✓ Resolved

Description

checkAccess() replaces a missing token before getAuthorId() passes it to the author-mapping
hooks as context.token. For token-less HTTP requests, plugins that handle a nullish token under
the documented hook contract instead receive a generated token and no longer take their
missing-token path.

Code

src/node/db/SecurityManager.ts[128]

+  if (token == null) token = padutils.generateAuthorToken();

Evidence

The author manager copies its token argument directly into the hook context, while the documented
hook contract says that value is nullish when no user token was supplied. The HTTP import route can
supply a missing cookie token to this changed call.

src/node/db/SecurityManager.ts[126-132]
src/node/db/AuthorManager.ts[145-154]
doc/api/hooks_server-side.md[219-238]
src/node/hooks/express/importexport.ts[97-106]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Generating a token before invoking the author-mapping hook changes its documented input for token-less requests, so plugins cannot identify those requests through `context.token`.
## Fix Focus Areas
- src/node/db/SecurityManager.ts[126-132]
- src/node/db/AuthorManager.ts[145-154]
- doc/api/hooks_server-side.md[219-238]
## Recommended Fix
Keep the original missing-token value available to author-mapping hooks while supplying a distinct generated database key for core's fallback mapping. Ensure the change cannot restore a shared null author.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Tip of the day

💡 Did you know, you can choose which labels appear on a finding, and whether they show icons or text

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗