remoteStorage protocol support by melvincarvalho · Pull Request #162 · JavaScriptSolidServer/JavaScriptSolidServer

melvincarvalho · 2026-03-10T05:19:19Z

Summary

Adds remoteStorage protocol (draft-dejong-remotestorage-22) to JSS
234 lines, no new dependencies — reuses existing filesystem storage, OAuth (OAuth 2.0 authorize/token flow #161), and WebFinger
Always on — no flag needed. Storage routes are just a path translation over existing infrastructure.

Endpoints

Method	Path	Description
GET	`/storage/:user/*`	Read file or folder listing
HEAD	`/storage/:user/*`	Metadata only
PUT	`/storage/:user/*`	Write file
DELETE	`/storage/:user/*`	Delete file

Features

Directory listings in RS JSON-LD format (http://remotestorage.io/spec/folder-description)
Public folder (/storage/:user/public/*) readable without auth
Conditional requests — ETags, If-Match, If-None-Match, 304 Not Modified
WebFinger discovery — RS link relation added to existing WebFinger response
Bearer token auth via OAuth flow from OAuth 2.0 authorize/token flow #161
WAC bypass — RS routes handle their own auth (Bearer tokens, not WAC)

How it works

RS path:     /storage/me/photos/vacation.jpg
             ↓ strip /storage/me
Storage:     /photos/vacation.jpg
             ↓ urlToPath()
Filesystem:  ./data/photos/vacation.jpg

Same files, same filesystem, same ETags. A remoteStorage.js client and a Solid client can read/write the same data.

WebFinger

{
  "rel": "http://tools.ietf.org/id/draft-dejong-remotestorage",
  "href": "https://example.com/storage/me",
  "properties": {
    "http://remotestorage.io/spec/version": "draft-dejong-remotestorage-22",
    "http://tools.ietf.org/html/rfc6749#section-4.2": "https://example.com/oauth/authorize",
    "http://tools.ietf.org/html/rfc6750#section-2.3": "Bearer"
  }
}

Test plan

GET /.well-known/webfinger?resource=acct:me@example.com includes RS link
GET /storage/me/ returns folder listing in RS JSON-LD format
PUT /storage/me/test/hello.txt creates file
GET /storage/me/test/hello.txt returns file with ETag
GET /storage/me/test/hello.txt with If-None-Match returns 304
DELETE /storage/me/test/hello.txt with If-Match deletes file
GET /storage/me/public/ works without auth
PUT /storage/me/test.txt without auth returns 401
remoteStorage.js client can connect via WebFinger discovery

Refs #106

Implements draft-dejong-remotestorage-22 on top of existing storage infrastructure. No new dependencies — reuses filesystem storage, OAuth flow (#161), and WebFinger. Endpoints: - GET /storage/:user/* — read file or folder (RS JSON-LD listing) - HEAD /storage/:user/* — metadata only - PUT /storage/:user/* — write file (with If-Match/If-None-Match) - DELETE /storage/:user/* — delete file (with If-Match) Features: - Public folder (/storage/:user/public/*) readable without auth - Conditional requests (ETags, If-Match, If-None-Match) - WebFinger discovery (RS link relation added to existing response) - Bearer token auth via existing OAuth flow - Always on — no flag needed Refs #106

Copilot

Pull request overview

Adds remoteStorage protocol endpoints to JavaScriptSolidServer (JSS) by introducing a Fastify plugin under /storage/:user/*, integrating Bearer-token auth, conditional requests, and WebFinger discovery.

Changes:

Register a new always-on remoteStoragePlugin and bypass WAC for /storage/* routes.
Add /storage/:user/* GET/HEAD/PUT/DELETE handlers backed by existing filesystem storage.
Extend the existing WebFinger response (in the ActivityPub plugin) with a remoteStorage link relation.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 12 comments.

File	Description
`src/server.js`	Registers the remoteStorage plugin and exempts `/storage/*` from the global WAC auth hook.
`src/remotestorage.js`	Implements remoteStorage HTTP endpoints, auth, directory listing, and conditional request behavior.
`src/ap/index.js`	Adds remoteStorage discovery information to the WebFinger response.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-10T05:24:09Z

+  // Register remoteStorage plugin (always on — no flag needed)
+  fastify.register(remoteStoragePlugin, {
+    username: apUsername || 'me',
+    ownerWebId: singleUser ? null : undefined  // single-user: any authenticated user; multi-user: check WebID


ownerWebId: singleUser ? null : undefined does not achieve the intended multi-user ownership check. In remoteStoragePlugin, const ownerWebId = options.ownerWebId || null coerces undefined to null, so the if (ownerWebId && webId !== ownerWebId) guard never runs in multi-user mode. Pass an explicit expected WebID here (derived from :user/pod) or change the plugin to distinguish “option omitted” from “option set to null”.

Suggested change

ownerWebId: singleUser ? null : undefined // single-user: any authenticated user; multi-user: check WebID

ownerWebId: singleUser ? null : (apUsername || 'me') // single-user: any authenticated user; multi-user: enforce owner WebID check

Copilot · 2026-03-10T05:24:10Z

+export async function remoteStoragePlugin (fastify, options = {}) {
+  const username = options.username || 'me'
+  const ownerWebId = options.ownerWebId || null
+
+  /**
+   * Extract the storage path from the URL
+   * /storage/me/photos/vacation.jpg → /photos/vacation.jpg
+   */
+  function getStoragePath (request) {
+    const wildcard = request.params['*'] || ''
+    return '/' + wildcard
+  }


username and the :user path param are never used to scope storage. getStoragePath() drops the /storage/:user prefix and operates on absolute paths like /photos/..., so requests to /storage/alice/* and /storage/bob/* hit the same underlying filesystem namespace. This breaks multi-user isolation and makes the :user portion of the API misleading; the handler should either enforce request.params.user === username (single-user) or map :user to a per-user prefix (multi-user/subdomain-aware).

Copilot · 2026-03-10T05:24:10Z

+    const { authorized, error } = await checkAuth(request, 'GET')
+    if (!authorized) {
+      return reply.code(401).header('WWW-Authenticate', 'Bearer').send({ error })


Authorization failures are always returned as 401 in the handlers, even when authentication succeeded but the user is not permitted (e.g., when checkAuth() returns error: 'Forbidden'). This makes clients treat an authorization failure as an authentication challenge. Return 403 when a WebID is present but access is denied, and reserve 401 (+ WWW-Authenticate) for missing/invalid credentials.

Suggested change

const { authorized, error } = await checkAuth(request, 'GET')

if (!authorized) {

return reply.code(401).header('WWW-Authenticate', 'Bearer').send({ error })

const { authorized, error, webId } = await checkAuth(request, 'GET')

if (!authorized) {

// Authentication failure: no WebID → 401 with WWW-Authenticate

if (!webId) {

return reply.code(401).header('WWW-Authenticate', 'Bearer').send({ error })

}

// Authorization failure: WebID present but access denied → 403

return reply.code(403).send({ error })

Copilot · 2026-03-10T05:24:11Z

+    // Conditional GET
+    const ifNoneMatch = request.headers['if-none-match']
+    if (ifNoneMatch && ifNoneMatch === info.etag) {
+      return reply.code(304).send()
+    }


Conditional request handling is too strict for real-world ETag headers: If-None-Match can include multiple values and weak validators (W/"..."), and equality against the raw info.etag will miss matches. The codebase already has checkIfNoneMatchForGet / checkIfMatch utilities (src/utils/conditional.js) that normalize/parse ETags; reusing them here would also align behavior with the existing LDP/DB handlers.

Copilot · 2026-03-10T05:24:11Z

+  // HEAD /storage/:user/* — metadata only
+  fastify.head('/storage/:user/*', async (request, reply) => {
+    const storagePath = getStoragePath(request)
+
+    const { authorized, error } = await checkAuth(request, 'HEAD')
+    if (!authorized) {
+      return reply.code(401).header('WWW-Authenticate', 'Bearer').send()
+    }
+
+    const info = await storage.stat(storagePath)
+    if (!info) {
+      return reply.code(404).send()
+    }
+
+    reply
+      .header('Content-Type', info.isDirectory ? 'application/ld+json' : getContentType(storagePath))
+      .header('ETag', info.etag)
+      .header('Cache-Control', 'no-cache')
+
+    if (!info.isDirectory) {
+      reply.header('Content-Length', info.size)
+    }
+
+    return reply.code(200).send()
+  })


HEAD does not implement conditional requests (If-None-Match / If-Match) even though remoteStorage clients commonly rely on them for metadata checks. As written, a HEAD with a matching If-None-Match will still return 200 rather than 304. Consider applying the same conditional logic used for GET (ideally via checkIfNoneMatchForGet).

Copilot · 2026-03-10T05:24:11Z

+    // Add remoteStorage link relation
+    response.links.push({
+      rel: 'http://tools.ietf.org/id/draft-dejong-remotestorage',
+      href: `${baseUrl}/storage/${config.username}`,


The WebFinger remoteStorage href is ${baseUrl}/storage/${config.username} (no trailing slash), but the server only registers /storage/:user/* routes. By analogy with the existing /* vs / handling in server.js, /storage/me is likely to 404 while /storage/me/ works. Either add explicit /storage/:user routes (or redirect) in the plugin, or include the trailing slash in the WebFinger href to match the registered routes.

Suggested change

href: `${baseUrl}/storage/${config.username}`,

href: `${baseUrl}/storage/${config.username}/`,

Copilot · 2026-03-10T05:24:12Z

        (gitEnabled && isGitRequest(request.url)) ||
        (activitypubEnabled && apPaths.some(p => request.url === p || request.url.startsWith(p + '?'))) ||
        isProfileAP ||
+        request.url.startsWith('/storage/') ||


Skipping the global WAC authorize() hook for all /storage/* requests means these routes fully bypass read-only mode, quota enforcement, and ACL protections unless the remoteStorage plugin re-implements them. Given the current plugin writes directly via storage.write/remove, this creates a path where clients can mutate data even when the server is configured read-only and without any WAC checks. Consider routing remoteStorage requests through the existing resource handlers (or reusing their quota/readOnly/notification logic) rather than bypassing WAC entirely at this layer.

Suggested change

request.url.startsWith('/storage/') ||

Copilot · 2026-03-10T05:24:12Z

+  function getStoragePath (request) {
+    const wildcard = request.params['*'] || ''
+    return '/' + wildcard
+  }
+
+  /**
+   * Check if request is authorized for the given method
+   * Public folder is readable without auth
+   */
+  async function checkAuth (request, method) {
+    const storagePath = getStoragePath(request)
+
+    // Public folder: readable without auth
+    if (storagePath.startsWith('/public/') && (method === 'GET' || method === 'HEAD')) {
+      return { authorized: true, webId: null }
+    }
+
+    const { webId, error } = await getWebIdFromRequestAsync(request)
+    if (!webId) {
+      return { authorized: false, webId: null, error: error || 'Unauthorized' }
+    }
+
+    // If ownerWebId is set, only the owner can access storage
+    if (ownerWebId && webId !== ownerWebId) {
+      return { authorized: false, webId, error: 'Forbidden' }
+    }
+
+    return { authorized: true, webId }
+  }


The remoteStorage routes operate directly on filesystem paths and currently allow access to dotfiles and server-internal control files under the data root (e.g. /.pods, /.account, /.acl, /.meta) because getStoragePath() can return those paths and checkAuth() only checks bearer authentication. This lets OAuth bearer tokens read/modify Solid ACLs and other administrative files, which is a serious privilege escalation vector. Restrict remoteStorage to a per-user root and explicitly deny any path segment starting with . (and/or specifically block .acl/.meta/.pods/.account).

Copilot · 2026-03-10T05:24:12Z

+    const content = await storage.read(storagePath)
+    if (content === null) {
+      return reply.code(404).send({ error: 'Not found' })
+    }
+
+    return reply
+      .header('Content-Type', getContentType(storagePath))
+      .header('Content-Length', content.length)
+      .header('ETag', info.etag)
+      .header('Cache-Control', 'no-cache')
+      .send(content)


GET reads entire files into memory via storage.read() and then sets Content-Length from the buffer. This can cause high memory usage for large remoteStorage objects and bypasses the server’s existing streaming + range support (storage.createReadStream() used elsewhere). Prefer streaming responses for file reads to keep memory bounded.

Suggested change

const content = await storage.read(storagePath)

if (content === null) {

return reply.code(404).send({ error: 'Not found' })

}

return reply

.header('Content-Type', getContentType(storagePath))

.header('Content-Length', content.length)

.header('ETag', info.etag)

.header('Cache-Control', 'no-cache')

.send(content)

const stream = storage.createReadStream(storagePath)

return reply

.header('Content-Type', getContentType(storagePath))

.header('Content-Length', info.size)

.header('ETag', info.etag)

.header('Cache-Control', 'no-cache')

.send(stream)

Copilot · 2026-03-10T05:24:12Z

+  // GET /storage/:user/* — read file or folder
+  fastify.get('/storage/:user/*', async (request, reply) => {
+    const storagePath = getStoragePath(request)
+
+    const { authorized, error } = await checkAuth(request, 'GET')
+    if (!authorized) {
+      return reply.code(401).header('WWW-Authenticate', 'Bearer').send({ error })
+    }
+
+    const info = await storage.stat(storagePath)
+    if (!info) {
+      return reply.code(404).send({ error: 'Not found' })
+    }
+
+    // Conditional GET
+    const ifNoneMatch = request.headers['if-none-match']
+    if (ifNoneMatch && ifNoneMatch === info.etag) {
+      return reply.code(304).send()
+    }
+
+    // Directory listing
+    if (info.isDirectory) {
+      const entries = await storage.listContainer(storagePath)
+      if (!entries) {
+        return reply.code(404).send({ error: 'Not found' })
+      }
+
+      const items = {}
+      for (const entry of entries) {
+        // Skip hidden files (ACLs, metadata)
+        if (entry.name.startsWith('.')) continue
+
+        const childPath = storagePath.endsWith('/') ? storagePath + entry.name : storagePath + '/' + entry.name
+        const childStat = await storage.stat(entry.isDirectory ? childPath + '/' : childPath)
+
+        if (entry.isDirectory) {
+          items[entry.name + '/'] = {
+            ETag: childStat?.etag?.replace(/"/g, '') || ''
+          }
+        } else {
+          items[entry.name] = {
+            ETag: childStat?.etag?.replace(/"/g, '') || '',
+            'Content-Type': getContentType(entry.name),
+            'Content-Length': childStat?.size || 0
+          }
+        }
+      }
+
+      return reply
+        .header('Content-Type', 'application/ld+json')
+        .header('ETag', info.etag)
+        .header('Cache-Control', 'no-cache')
+        .send({
+          '@context': 'http://remotestorage.io/spec/folder-description',
+          items
+        })
+    }
+
+    // File
+    const content = await storage.read(storagePath)
+    if (content === null) {
+      return reply.code(404).send({ error: 'Not found' })
+    }
+
+    return reply
+      .header('Content-Type', getContentType(storagePath))
+      .header('Content-Length', content.length)
+      .header('ETag', info.etag)
+      .header('Cache-Control', 'no-cache')
+      .send(content)
+  })
+
+  // HEAD /storage/:user/* — metadata only
+  fastify.head('/storage/:user/*', async (request, reply) => {
+    const storagePath = getStoragePath(request)
+
+    const { authorized, error } = await checkAuth(request, 'HEAD')
+    if (!authorized) {
+      return reply.code(401).header('WWW-Authenticate', 'Bearer').send()
+    }
+
+    const info = await storage.stat(storagePath)
+    if (!info) {
+      return reply.code(404).send()
+    }
+
+    reply
+      .header('Content-Type', info.isDirectory ? 'application/ld+json' : getContentType(storagePath))
+      .header('ETag', info.etag)
+      .header('Cache-Control', 'no-cache')
+
+    if (!info.isDirectory) {
+      reply.header('Content-Length', info.size)
+    }
+
+    return reply.code(200).send()
+  })
+
+  // PUT /storage/:user/* — write file
+  fastify.put('/storage/:user/*', async (request, reply) => {
+    const storagePath = getStoragePath(request)
+
+    const { authorized, error } = await checkAuth(request, 'PUT')
+    if (!authorized) {
+      return reply.code(401).header('WWW-Authenticate', 'Bearer').send({ error })
+    }
+
+    // Directories end with / — can't PUT to a directory
+    if (storagePath.endsWith('/')) {
+      return reply.code(400).send({ error: 'Cannot PUT to a folder path' })
+    }
+
+    // Conditional write
+    const ifMatch = request.headers['if-match']
+    const ifNoneMatch = request.headers['if-none-match']
+    const existing = await storage.stat(storagePath)
+
+    if (ifMatch && (!existing || existing.etag !== ifMatch)) {
+      return reply.code(412).send({ error: 'Precondition failed' })
+    }
+    if (ifNoneMatch === '*' && existing) {
+      return reply.code(412).send({ error: 'Resource already exists' })
+    }
+
+    const content = Buffer.isBuffer(request.body) ? request.body : Buffer.from(request.body || '')
+    const success = await storage.write(storagePath, content)
+    if (!success) {
+      return reply.code(500).send({ error: 'Write failed' })
+    }
+
+    const newStat = await storage.stat(storagePath)
+    const statusCode = existing ? 200 : 201
+
+    return reply
+      .code(statusCode)
+      .header('ETag', newStat?.etag || '')
+      .send()
+  })
+
+  // DELETE /storage/:user/* — delete file
+  fastify.delete('/storage/:user/*', async (request, reply) => {
+    const storagePath = getStoragePath(request)
+
+    const { authorized, error } = await checkAuth(request, 'DELETE')
+    if (!authorized) {
+      return reply.code(401).header('WWW-Authenticate', 'Bearer').send({ error })
+    }
+
+    const existing = await storage.stat(storagePath)
+    if (!existing) {
+      return reply.code(404).send({ error: 'Not found' })
+    }
+
+    // Conditional delete
+    const ifMatch = request.headers['if-match']
+    if (ifMatch && existing.etag !== ifMatch) {
+      return reply.code(412).send({ error: 'Precondition failed' })
+    }
+
+    const success = await storage.remove(storagePath)
+    if (!success) {
+      return reply.code(500).send({ error: 'Delete failed' })
+    }
+
+    return reply
+      .code(200)
+      .header('ETag', existing.etag)
+      .send()
+  })


There are comprehensive Node.js tests in test/*.test.js, but this PR introduces a new protocol surface (/storage/:user/* methods, auth rules, directory listing format, conditional requests) without any automated tests. Adding a focused test file (e.g., covering WebFinger discovery, public folder unauthenticated GET/HEAD, and If-Match/If-None-Match behavior) would help prevent regressions.

- Enforce username match (request.params.user === configured username) - Return 403 (not 401) when authenticated but forbidden - Use shared conditional utilities (checkIfMatch, checkIfNoneMatchForGet/Write) - Block dotfile access (.acl, .meta, etc.) — security fix - Use createReadStream() for file reads instead of buffering entire files - Add conditional request handling to HEAD - Respect readOnly mode for PUT/DELETE - Fix WebFinger RS href trailing slash

Document the three new features from PRs #159, #161, #162: - Mastodon-compatible API endpoints and OAuth 2.0 flow - remoteStorage protocol (always on, no flag needed) - Updated project structure with new files

Adds Mastodon API, OAuth 2.0, and remoteStorage protocol. Also fixes stale 0.0.67 version strings in nodeinfo and instance endpoint.

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 10 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-10T05:36:13Z

+  // Register remoteStorage plugin (always on — no flag needed)
+  fastify.register(remoteStoragePlugin, {
+    username: apUsername || 'me',
+    ownerWebId: singleUser ? null : undefined  // single-user: any authenticated user; multi-user: check WebID
+  });


remoteStoragePlugin is registered with username: apUsername || 'me', which is unrelated to singleUserName and can diverge from the actual pod path the server creates/serves. Additionally, passing ownerWebId: singleUser ? null : undefined can't express the intended difference because the plugin currently normalizes falsy values to null. Consider wiring remoteStorage username/ownership to the pod/user model explicitly (e.g., singleUserName in single-user mode, and per-request user mapping in multi-user mode).

Copilot · 2026-03-10T05:36:14Z

+      .header('Content-Length', info.size)
+      .header('ETag', info.etag)
+      .header('Cache-Control', 'no-cache')
+      .send(result.stream)
+  })
+
+  // HEAD /storage/:user/* — metadata only
+  fastify.head('/storage/:user/*', async (request, reply) => {


PUT handler doesn't currently honor the server's read-only mode (request.config.readOnly), unlike other write endpoints which return 405 when read-only is enabled. Since /storage/* bypasses the WAC preHandler, this route needs its own read-only enforcement.

Copilot · 2026-03-10T05:36:14Z

+    }
+
+    return reply.code(200).send()
+  })
+
+  // PUT /storage/:user/* — write file
+  fastify.put('/storage/:user/*', async (request, reply) => {
+    if (!checkUsername(request, reply)) return


DELETE handler doesn't currently honor the server's read-only mode (request.config.readOnly), unlike other write endpoints. Add a read-only check here as well to prevent deletes when the server is configured read-only.

Copilot · 2026-03-10T05:36:14Z

+    const storagePath = getStoragePath(request)
+
+    if (hasDotfile(storagePath)) {
+      return reply.code(403).send({ error: 'Cannot write to dotfiles' })
+    }


Conditional DELETE uses a direct existing.etag !== ifMatch check, which doesn't handle standard If-Match parsing/normalization (weak ETags, multiple values, quoted/unquoted). Using the shared conditional helpers would align behavior with the rest of the server.

Copilot · 2026-03-10T05:36:14Z

+
+      const items = {}
+      for (const entry of entries) {
+        // Skip dotfiles (ACLs, metadata, etc.)
+        if (entry.name.startsWith('.')) continue
+
+        const childPath = storagePath.endsWith('/') ? storagePath + entry.name : storagePath + '/' + entry.name


GET for files reads the entire file into memory via storage.read() before sending it. For large files this can create significant memory pressure; the filesystem storage layer already exposes createReadStream(), which would allow streaming responses like the main resource handler.

Copilot · 2026-03-10T05:36:14Z

+    // Add remoteStorage link relation
+    response.links.push({
+      rel: 'http://tools.ietf.org/id/draft-dejong-remotestorage',
+      href: `${baseUrl}/storage/${config.username}/`,
+      properties: {


The WebFinger response advertises storage at /storage/${config.username} regardless of the acct: username requested. This prevents multi-user discovery and can mislead clients if the requested acct doesn't match config.username. Consider using the parsed WebFinger username in the storage href (and ensuring the storage routes validate/authorize that user appropriately).

Copilot · 2026-03-10T05:36:14Z

+      rel: 'http://tools.ietf.org/id/draft-dejong-remotestorage',
+      href: `${baseUrl}/storage/${config.username}/`,
+      properties: {


The advertised remoteStorage href does not include a trailing slash, while storage routes are defined as /storage/:user/* (which may not match /storage/:user without /). To avoid clients hitting a 404 when dereferencing the advertised URL, either advertise /storage/:user/ or add a handler/redirect for /storage/:user.

Copilot · 2026-03-10T05:36:15Z

+   * Public folder is readable without auth
+   */


This PR adds new externally-consumed endpoints (/storage/:user/*) with non-trivial behavior (auth, public folder, conditional requests). The repo has an extensive test suite, but there are currently no tests for these routes; please add integration tests covering at least GET/PUT/DELETE/HEAD, public access, and conditional headers (If-Match / If-None-Match).

Copilot · 2026-03-10T05:36:15Z

+   * Public folder is readable without auth
+   */
+  async function checkAuth (request, method) {
+    const storagePath = getStoragePath(request)


remoteStorage routes currently allow direct access to dotfiles under the data root (e.g., requesting /storage/<user>/.pods, /.acl, /.meta, etc.) because no dotfile/path-segment filtering is applied before calling storage.stat/read. Since some dotfiles are intentionally accessible for Solid internals, remoteStorage should independently block dotfiles (typically any segment starting with .) to avoid leaking server/pod metadata.

Copilot · 2026-03-10T05:36:15Z

+   */
+  function hasDotfile (storagePath) {
+    const segments = storagePath.split('/')
+    return segments.some(s => s.startsWith('.') && s.length > 1)


checkAuth() returns 'Forbidden' when ownerWebId is set and the token WebID doesn't match, but the handlers always respond with HTTP 401. Please propagate a 403 status for authorization failures (and only set WWW-Authenticate: Bearer for 401), so clients can distinguish unauthenticated vs forbidden.

The RS plugin username should match the pod username, not the ActivityPub username which may differ or be unset.

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 10 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-10T05:49:46Z

+      const items = {}
+      for (const entry of entries) {
+        // Skip dotfiles (ACLs, metadata, etc.)
+        if (entry.name.startsWith('.')) continue
+
+        const childPath = storagePath.endsWith('/') ? storagePath + entry.name : storagePath + '/' + entry.name
+        const childStat = await storage.stat(entry.isDirectory ? childPath + '/' : childPath)
+


Directory listing builds per-entry metadata by await storage.stat(...) inside a for loop, resulting in N+1 sequential filesystem stats for large folders. This can become noticeably slow for directories with many entries. Consider batching these stats (e.g., Promise.all) or extending listContainer() to return the required metadata to avoid repeated stat calls.

Copilot · 2026-03-10T05:49:47Z

+// Dotfiles that must never be exposed via remoteStorage
+const BLOCKED_NAMES = new Set(['.acl', '.meta', '.pods'])
+


BLOCKED_NAMES is declared but never used. Since dotfile blocking is implemented via hasDotfile(), this constant is dead code and may confuse future maintenance. Consider removing it or using it to explicitly block only specific dotfiles (if that was the intent).

Copilot · 2026-03-10T05:49:47Z

+    const { webId, error } = await getWebIdFromRequestAsync(request)
+    if (!webId) {
+      return { authorized: false, webId: null, error: error || 'Unauthorized', status: 401 }
+    }
+
+    // If ownerWebId is set, only the owner can access storage
+    if (ownerWebId && webId !== ownerWebId) {
+      return { authorized: false, webId, error: 'Forbidden', status: 403 }
+    }
+
+    return { authorized: true, webId }
+  }


checkAuth() treats any valid token as authorized when ownerWebId is unset, and does not verify that the authenticated webId corresponds to request.params.user. Combined with the always-on registration, this allows authenticated users to access another user’s storage path in multi-user setups. Consider enforcing webId ↔ :user ownership (or always configuring ownerWebId for single-user) so the storage namespace can’t be accessed by other accounts.

Copilot · 2026-03-10T05:49:47Z

+    const success = await storage.write(storagePath, content)
+    if (!success) {
+      return reply.code(500).send({ error: 'Write failed' })
+    }


remoteStorage PUT writes directly via storage.write() without applying the pod quota checks / quota accounting used by the main LDP handlePut path. This allows clients to exceed configured quotas (and bypass any related operational constraints). Consider reusing the existing PUT/DELETE handlers or invoking checkQuota/updateQuotaUsage equivalents for these routes.

Copilot · 2026-03-10T05:49:47Z

+  // PUT /storage/:user/* — write file
+  fastify.put('/storage/:user/*', async (request, reply) => {
+    if (!checkUsername(request, reply)) return
+
+    // Respect readOnly mode
+    if (request.config?.readOnly) {
+      return reply.code(405).send({ error: 'Method Not Allowed', message: 'Server is in read-only mode' })
+    }
+
+    const storagePath = getStoragePath(request)
+
+    if (hasDotfile(storagePath)) {
+      return reply.code(403).send({ error: 'Cannot write to dotfiles' })
+    }
+
+    const { authorized, error, status } = await checkAuth(request, 'PUT')
+    if (!authorized) {
+      const code = status || 401
+      if (code === 401) reply.header('WWW-Authenticate', 'Bearer')
+      return reply.code(code).send({ error })
+    }
+
+    // Directories end with / — can't PUT to a directory
+    if (storagePath.endsWith('/')) {
+      return reply.code(400).send({ error: 'Cannot PUT to a folder path' })
+    }
+
+    // Conditional write — use shared utilities
+    const existing = await storage.stat(storagePath)
+
+    const ifMatchResult = checkIfMatch(request.headers['if-match'], existing?.etag || null)
+    if (!ifMatchResult.ok) {
+      return reply.code(ifMatchResult.status).send({ error: ifMatchResult.error })
+    }
+
+    const ifNoneMatchResult = checkIfNoneMatchForWrite(request.headers['if-none-match'], existing?.etag || null)
+    if (!ifNoneMatchResult.ok) {
+      return reply.code(ifNoneMatchResult.status).send({ error: ifNoneMatchResult.error })
+    }
+
+    const content = Buffer.isBuffer(request.body) ? request.body : Buffer.from(request.body || '')
+    const success = await storage.write(storagePath, content)
+    if (!success) {
+      return reply.code(500).send({ error: 'Write failed' })
+    }
+
+    const newStat = await storage.stat(storagePath)
+    const statusCode = existing ? 200 : 201
+
+    return reply
+      .code(statusCode)
+      .header('ETag', newStat?.etag || '')
+      .send()
+  })
+
+  // DELETE /storage/:user/* — delete file
+  fastify.delete('/storage/:user/*', async (request, reply) => {
+    if (!checkUsername(request, reply)) return
+
+    // Respect readOnly mode
+    if (request.config?.readOnly) {
+      return reply.code(405).send({ error: 'Method Not Allowed', message: 'Server is in read-only mode' })
+    }
+


Write operations under /* are rate-limited via writeRateLimit, but the new /storage/:user/* PUT/DELETE routes are not. This creates an easy bypass for write-abuse/resource-exhaustion protections. Consider applying the same rateLimit config (keyed by webId/IP) to these remoteStorage write routes.

Copilot · 2026-03-10T05:49:47Z

+    // Add remoteStorage link relation
+    response.links.push({
+      rel: 'http://tools.ietf.org/id/draft-dejong-remotestorage',
+      href: `${baseUrl}/storage/${config.username}/`,
+      properties: {
+        'http://remotestorage.io/spec/version': 'draft-dejong-remotestorage-22',
+        'http://tools.ietf.org/html/rfc6749#section-4.2': `${baseUrl}/oauth/authorize`,
+        'http://tools.ietf.org/html/rfc6750#section-2.3': 'Bearer'
+      }
+    })


The remoteStorage discovery link is added inside activityPubPlugin’s WebFinger handler. Since activitypub is disabled by default (and /oauth/* is also registered only in this plugin), remoteStorage clients won’t be able to discover the storage endpoint or complete the OAuth flow unless ActivityPub is enabled—contradicting the PR/README claim that remoteStorage is “always on, no flag needed”. Consider moving the WebFinger + OAuth pieces to an always-on plugin, or update the docs/description to reflect the --activitypub requirement.

Copilot · 2026-03-10T05:49:48Z

+  // GET /storage/:user/* — read file or folder
+  fastify.get('/storage/:user/*', async (request, reply) => {
+    if (!checkUsername(request, reply)) return
+
+    const storagePath = getStoragePath(request)
+
+    // Block dotfile access
+    if (hasDotfile(storagePath)) {
+      return reply.code(404).send({ error: 'Not found' })
+    }
+
+    const { authorized, error, status } = await checkAuth(request, 'GET')
+    if (!authorized) {
+      const code = status || 401
+      if (code === 401) reply.header('WWW-Authenticate', 'Bearer')
+      return reply.code(code).send({ error })
+    }
+
+    const info = await storage.stat(storagePath)
+    if (!info) {
+      return reply.code(404).send({ error: 'Not found' })
+    }
+
+    // Conditional GET — use shared utility
+    const cond = checkIfNoneMatchForGet(request.headers['if-none-match'], info.etag)
+    if (!cond.ok) {
+      return reply.code(304).send()
+    }


This PR adds a new protocol surface area (auth + conditional requests + filesystem writes) but does not add automated tests. Given the repo uses node --test with extensive integration coverage, consider adding tests that cover at least: WebFinger RS link, public read access, Bearer-required writes, and conditional GET/PUT/DELETE behaviors (If-None-Match/If-Match).

Copilot · 2026-03-10T05:49:48Z

        (activitypubEnabled && apPaths.some(p => request.url === p || request.url.startsWith(p + '?'))) ||
        isProfileAP ||
+        request.url.startsWith('/storage/') ||
        (mongoEnabled && (request.url === '/db' || request.url.startsWith('/db/'))) ||


WAC auth is skipped for any URL starting with /storage/. Given these routes implement their own auth, it’s important they provide equivalent protections to WAC for writes (e.g., quota enforcement, rate limiting, and restricting writes to the owning pod). Right now remoteStorage handlers write directly to filesystem storage, so this skip can become a privilege-escalation path unless the plugin enforces the same constraints.

Copilot · 2026-03-10T05:49:48Z

+  function getStoragePath (request) {
+    const wildcard = request.params['*'] || ''
+    return '/' + wildcard
+  }


getStoragePath() ignores the :user segment entirely (/storage/:user/* → /*). This means remoteStorage requests do not map into the user’s existing pod path (e.g., /me/...) and can create/read resources outside the intended user namespace, undermining the “same files as Solid” claim and potentially exposing/overwriting server resources. Consider including the resolved pod/user prefix in the translated path (and making it subdomain-aware if needed).

Copilot · 2026-03-10T05:49:48Z

+    const content = Buffer.isBuffer(request.body) ? request.body : Buffer.from(request.body || '')
+    const success = await storage.write(storagePath, content)


PUT body handling can throw: when Fastify parses application/json, request.body is an object, but Buffer.from(request.body || '') will raise a TypeError. Consider mirroring the existing LDP handlePut behavior (Buffer/string/object cases) so remoteStorage can store JSON safely without crashing the request handler.

Storage routes are always on, but WebFinger and OAuth are currently inside the AP plugin. Tracked in #164 for standalone extraction.

melvincarvalho requested a review from Copilot March 10, 2026 05:19

Copilot started reviewing on behalf of melvincarvalho March 10, 2026 05:20 View session

Copilot AI reviewed Mar 10, 2026

View reviewed changes

melvincarvalho requested a review from Copilot March 10, 2026 05:30

Copilot started reviewing on behalf of melvincarvalho March 10, 2026 05:31 View session

melvincarvalho added 2 commits March 10, 2026 06:33

Add Mastodon API, OAuth 2.0, and remoteStorage docs to README

aff948a

Document the three new features from PRs #159, #161, #162: - Mastodon-compatible API endpoints and OAuth 2.0 flow - remoteStorage protocol (always on, no flag needed) - Updated project structure with new files

Bump version to 0.0.97

7548004

Adds Mastodon API, OAuth 2.0, and remoteStorage protocol. Also fixes stale 0.0.67 version strings in nodeinfo and instance endpoint.

Copilot AI reviewed Mar 10, 2026

View reviewed changes

Wire remoteStorage username to singleUserName, not apUsername

e704adc

The RS plugin username should match the pod username, not the ActivityPub username which may differ or be unset.

melvincarvalho mentioned this pull request Mar 10, 2026

remoteStorage: deferred improvements from PR #162 review #163

Open

melvincarvalho requested a review from Copilot March 10, 2026 05:38

Copilot started reviewing on behalf of melvincarvalho March 10, 2026 05:38 View session

Copilot AI reviewed Mar 10, 2026

View reviewed changes

melvincarvalho added 2 commits March 10, 2026 06:50

Remove unused BLOCKED_NAMES constant

33e069b

Update docs: RS requires --activitypub for discovery and OAuth

a289965

Storage routes are always on, but WebFinger and OAuth are currently inside the AP plugin. Tracked in #164 for standalone extraction.

melvincarvalho mentioned this pull request Mar 10, 2026

Extract WebFinger and OAuth into standalone always-on plugins #164

Open

melvincarvalho merged commit 127b6ba into gh-pages Mar 10, 2026

melvincarvalho deleted the feature/remotestorage branch March 10, 2026 05:53

	ownerWebId: singleUser ? null : undefined // single-user: any authenticated user; multi-user: check WebID
	ownerWebId: singleUser ? null : (apUsername \|\| 'me') // single-user: any authenticated user; multi-user: enforce owner WebID check

	href: `${baseUrl}/storage/${config.username}`,
	href: `${baseUrl}/storage/${config.username}/`,

		// Dotfiles that must never be exposed via remoteStorage
		const BLOCKED_NAMES = new Set(['.acl', '.meta', '.pods'])

		const content = Buffer.isBuffer(request.body) ? request.body : Buffer.from(request.body \|\| '')
		const success = await storage.write(storagePath, content)

Conversation

melvincarvalho commented Mar 10, 2026

Summary

Endpoints

Features

How it works

WebFinger

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment