René's URL Explorer Experiment

{"@context":"https://schema.org","@type":"DiscussionForumPosting","headline":"Roadmap: Production Readiness for v0.1 Release - Code Quality \u0026 Cleanup","articleBody":"## Summary\n\nThis issue tracks all code quality improvements, refactoring, security fixes, and cleanup needed to achieve a production-ready **v0.1 release** of JavaScriptSolidServer. Based on a comprehensive deep-dive analysis of 47+ source files.\n\n**Current Version**: 0.0.81  \n**Target Version**: 0.1.0  \n**Overall Readiness**: ~65%\n\n---\n\n## Executive Summary\n\n| Category | Critical | High | Medium | Low |\n|----------|----------|------|--------|-----|\n| Security | 1 | 1 | 5 | 0 |\n| Error Handling | 0 | 1 | 3 | 1 |\n| Code Quality | 0 | 0 | 5 | 4 |\n| Performance | 0 | 1 | 2 | 2 |\n| Testing | 0 | 0 | 3 | 1 |\n| Documentation | 0 | 0 | 3 | 1 |\n| Architecture | 0 | 0 | 3 | 1 |\n| **Total** | **1** | **3** | **24** | **10** |\n\n---\n\n## 🚨 CRITICAL - Must Fix (Blockers)\n\n### 1. Path Traversal Vulnerability in Git Handler\n**File**: `src/handlers/git.js` lines 56-61  \n**Severity**: CRITICAL  \n**Status**: UNFIXED\n\n```javascript\n// VULNERABLE: Uses path.resolve without urlToPath sanitization\nconst repoAbs = resolve(dataRoot, repoRelative);\n```\n\n**Issue**: Git handler bypasses path traversal protection used elsewhere. An attacker could potentially access files outside the data directory.\n\n**Fix**: Use `urlToPath()` from `/src/utils/url.js` which includes traversal protection:\n```javascript\nimport { urlToPath } from '../utils/url.js';\nconst repoAbs = urlToPath(repoRelative);\n```\n\n---\n\n### 2. Unimplemented Agent Group Authorization\n**File**: `src/wac/checker.js` line 182  \n**Severity**: CRITICAL  \n**Status**: UNFIXED\n\n```javascript\n// TODO: Check agent groups\n```\n\n**Issue**: WAC `agentGroup` authorization is silently ignored. Users expecting group-based access control will have security issues.\n\n**Options**:\n1. Implement agent group checking (proper fix)\n2. Return 501 Not Implemented when agentGroup encountered (temporary)\n3. Log warning when agentGroup ignored (minimum)\n\n---\n\n### 3. WebSocket Subscription Memory Leak\n**File**: `src/notifications/websocket.js` lines 28-48  \n**Severity**: HIGH  \n**Status**: UNFIXED\n\n```javascript\nconst subscriptions = new Map();  // WebSocket -\u003e Set\u003curl\u003e\nconst subscribers = new Map();    // url -\u003e Set\u003cWebSocket\u003e\n```\n\n**Issue**: No cleanup when WebSocket closes unexpectedly. Maps grow unbounded if close events don't fire properly.\n\n**Fix**: Add explicit cleanup in close handler and periodic garbage collection:\n```javascript\nsocket.on('close', () =\u003e {\n  const urls = subscriptions.get(socket);\n  if (urls) {\n    for (const url of urls) {\n      subscribers.get(url)?.delete(socket);\n      if (subscribers.get(url)?.size === 0) {\n        subscribers.delete(url);\n      }\n    }\n    subscriptions.delete(socket);\n  }\n});\n```\n\n---\n\n### 4. JSON.parse DoS Vulnerabilities\n**Files**: Multiple (5 locations)  \n**Severity**: HIGH  \n**Status**: PARTIAL\n\n`safeJsonParse()` exists in `src/utils/json.js` but not used consistently:\n\n| File | Line | Status |\n|------|------|--------|\n| `src/wac/parser.js` | 40 | ❌ Raw JSON.parse |\n| `src/auth/did-nostr.js` | 129, 141 | ❌ Raw JSON.parse |\n| `src/nostr/relay.js` | 242 | ❌ Raw JSON.parse |\n| `src/auth/token.js` | 105 | ❌ Raw JSON.parse |\n| `src/ap/routes/inbox.js` | 126 | ❌ Raw JSON.parse |\n\n**Fix**: Replace all `JSON.parse()` with `safeJsonParse()` that has size limits and try/catch.\n\n---\n\n## ⚠️ HIGH PRIORITY - Should Fix Before v0.1\n\n### 5. Silent Error Swallowing in Storage\n**File**: `src/storage/filesystem.js` lines 34, 49, 50+  \n**Severity**: MEDIUM\n\n```javascript\nexport async function read(urlPath) {\n  try {\n    return await fs.readFile(filePath);\n  } catch {\n    return null;  // Can't distinguish \"not found\" from \"permission error\"\n  }\n}\n```\n\n**Issue**: Callers cannot distinguish between:\n- File not found (404)\n- Permission denied (403)\n- Disk error (500)\n\n**Fix**: Return error objects or throw typed errors:\n```javascript\nexport async function read(urlPath) {\n  try {\n    return { data: await fs.readFile(filePath) };\n  } catch (err) {\n    if (err.code === 'ENOENT') return { error: 'not_found' };\n    if (err.code === 'EACCES') return { error: 'permission_denied' };\n    return { error: 'system_error', details: err.message };\n  }\n}\n```\n\n---\n\n### 6. Inconsistent Error Response Format\n**Files**: Multiple handlers  \n**Severity**: MEDIUM\n\nCurrent formats vary:\n\n```javascript\n// Format A - Minimal\n{ error: 'Not Found' }\n\n// Format B - With message\n{ error: 'Forbidden', message: 'Dotfile access is not allowed' }\n\n// Format C - Extended\n{ error: 'Too Many Requests', message: '...', retryAfter: 60 }\n```\n\n**Fix**: Standardize all error responses:\n```javascript\n{\n  error: 'ErrorCode',\n  message: 'Human readable description',\n  statusCode: 404,\n  details: { /* optional context */ }\n}\n```\n\nCreate `src/utils/errors.js`:\n```javascript\nexport function errorResponse(reply, code, error, message, details = {}) {\n  return reply.code(code).send({\n    error,\n    message,\n    statusCode: code,\n    ...details\n  });\n}\n```\n\n---\n\n### 7. Missing Global Security Headers\n**File**: `src/server.js`  \n**Severity**: MEDIUM\n\nSecurity headers only applied to mashlib responses, not globally.\n\n**Fix**: Add global `onSend` hook:\n```javascript\nfastify.addHook('onSend', async (request, reply) =\u003e {\n  reply.header('X-Content-Type-Options', 'nosniff');\n  reply.header('X-Frame-Options', 'DENY');\n  reply.header('X-XSS-Protection', '1; mode=block');\n  reply.header('Referrer-Policy', 'strict-origin-when-cross-origin');\n});\n```\n\n---\n\n### 8. Request Correlation IDs\n**Files**: All handlers  \n**Severity**: MEDIUM\n\nNo request IDs for tracing requests through logs.\n\n**Fix**: Add request ID generation:\n```javascript\nfastify.addHook('onRequest', async (request) =\u003e {\n  request.id = request.headers['x-request-id'] || crypto.randomUUID();\n  request.log = request.log.child({ requestId: request.id });\n});\n\nfastify.addHook('onSend', async (request, reply) =\u003e {\n  reply.header('X-Request-ID', request.id);\n});\n```\n\n---\n\n## 🔧 MEDIUM PRIORITY - Nice to Fix\n\n### 9. Code Duplication - `getRequestPaths()`\n**Files**: `src/handlers/resource.js:24-31`, `src/handlers/container.js:15-22`  \n**Severity**: LOW\n\nNearly identical implementations.\n\n**Fix**: Extract to `src/utils/paths.js`:\n```javascript\nexport function getRequestPaths(request, dataRoot) {\n  const urlPath = decodeURIComponent(request.url.split('?')[0]);\n  const fullPath = urlToPath(urlPath);\n  return { urlPath, fullPath };\n}\n```\n\n---\n\n### 10. Magic Numbers/Strings\n**Files**: Multiple  \n**Severity**: LOW\n\nCreate `src/constants.js`:\n```javascript\nexport const LIMITS = {\n  SLUG_MAX_LENGTH: 255,\n  BODY_MAX_SIZE: 10 * 1024 * 1024,  // 10MB\n  WS_MAX_SUBSCRIPTIONS: 100,\n  WS_MAX_URL_LENGTH: 2048,\n};\n\nexport const CACHE = {\n  JTI_CLEANUP_INTERVAL: 60 * 1000,\n  JTI_TTL: 15 * 60 * 1000,\n  DPOP_MAX_AGE: 5 * 60,\n};\n\nexport const RATE_LIMITS = {\n  GLOBAL_PER_MINUTE: 100,\n  WRITE_PER_MINUTE: 60,\n  POD_CREATION_PER_DAY: 1,\n};\n```\n\n---\n\n### 11. Complex Functions Need Refactoring\n**Severity**: MEDIUM\n\n| Function | File | Lines | Issue |\n|----------|------|-------|-------|\n| `createServer()` | `src/server.js` | 540 | God function, too many responsibilities |\n| `handleGet()` | `src/handlers/resource.js` | 150+ | Multiple concerns mixed |\n| `authorize()` | `src/auth/middleware.js` | 80+ | Complex nested conditionals |\n\n**Recommendation**: Break into smaller focused functions. Example for `createServer()`:\n```javascript\nexport async function createServer(options) {\n  const fastify = createFastifyInstance(options);\n  configureRequestContext(fastify, options);\n  registerSecurityPlugins(fastify, options);\n  registerFeaturePlugins(fastify, options);\n  registerRoutes(fastify, options);\n  return fastify;\n}\n```\n\n---\n\n### 12. Dead Code Removal\n**File**: `src/utils/url.js` line 11\n\n```javascript\nexport let DATA_ROOT = './data';  // Legacy, superseded by getDataRoot()\n```\n\n**Fix**: Remove legacy export, use only `getDataRoot()` function.\n\n---\n\n### 13. ACL Lookup Caching\n**File**: `src/wac/checker.js` lines 74-98  \n**Severity**: LOW (Performance)\n\n```javascript\nwhile (currentStoragePath \u0026\u0026 currentStoragePath !== '/') {\n  // Linear walk up hierarchy for every request - no caching\n}\n```\n\n**Fix**: Add LRU cache for parsed ACL rules:\n```javascript\nimport { LRUCache } from 'lru-cache';\n\nconst aclCache = new LRUCache({ max: 500, ttl: 5 * 60 * 1000 });\n\nasync function getAcl(path) {\n  const cached = aclCache.get(path);\n  if (cached) return cached;\n  const acl = await loadAndParseAcl(path);\n  aclCache.set(path, acl);\n  return acl;\n}\n```\n\n---\n\n## 📝 DOCUMENTATION\n\n### 14. Missing JSDoc Comments\n**Priority**: MEDIUM\n\n| File | Status |\n|------|--------|\n| `src/storage/filesystem.js` | ❌ Most functions undocumented |\n| `src/handlers/resource.js` | ⚠️ Partial |\n| `src/wac/parser.js` | ❌ Complex ACL parsing undocumented |\n| `src/ap/routes/inbox.js` | ❌ ActivityPub flow undocumented |\n| `src/auth/nostr.js` | ❌ Schnorr verification undocumented |\n\n---\n\n### 15. Undocumented API Behaviors\n**Priority**: MEDIUM\n\nDocument in README or API docs:\n- [ ] `POST /.pods` endpoint request/response format\n- [ ] IdP issuer URL trailing slash handling\n- [ ] Subdomain mode pod URL structure\n- [ ] WebSocket protocol version (solid-0.1) compatibility\n- [ ] Rate limiting headers and retry behavior\n\n---\n\n## 🧪 TESTING\n\n### 16. Test Coverage Gaps\n**Priority**: MEDIUM\n\n| Area | Status | Priority |\n|------|--------|----------|\n| Git handler (`src/handlers/git.js`) | ❌ No tests | HIGH |\n| Range requests (streaming) | ❌ No tests | MEDIUM |\n| ActivityPub federation | ❌ Limited | MEDIUM |\n| Nostr relay stress tests | ❌ None | LOW |\n| Concurrent request handling | ❌ None | MEDIUM |\n| WebID-TLS auth | ❌ No tests | LOW |\n\n---\n\n### 17. Missing Integration Tests\n**Priority**: MEDIUM\n\n- [ ] Auth + WAC + Resource access combined\n- [ ] Quota enforcement during multi-file operations\n- [ ] IdP registration + pod access flow\n- [ ] Concurrent modification conflicts\n\n---\n\n## 🏗️ ARCHITECTURE (Post-v0.1)\n\n### 18. Storage Abstraction\n**Priority**: LOW (v0.2)\n\nCurrently hardcoded to filesystem. Abstract for future backends:\n\n```javascript\n// src/storage/interface.js\nexport interface StorageAdapter {\n  read(path: string): Promise\u003cBuffer | null\u003e;\n  write(path: string, data: Buffer): Promise\u003cvoid\u003e;\n  delete(path: string): Promise\u003cvoid\u003e;\n  exists(path: string): Promise\u003cboolean\u003e;\n  stat(path: string): Promise\u003cStats\u003e;\n  list(path: string): Promise\u003cstring[]\u003e;\n}\n\n// src/storage/filesystem.js\nexport class FilesystemAdapter implements StorageAdapter { ... }\n\n// Future: src/storage/s3.js, src/storage/sqlite.js\n```\n\n---\n\n### 19. Request Context Object\n**Priority**: LOW (v0.2)\n\nCurrent approach decorates Fastify request with 15+ properties. Create proper context:\n\n```javascript\n// Instead of request.connegEnabled, request.mashlibEnabled, etc.\nclass RequestContext {\n  constructor(request, config) {\n    this.config = config;\n    this.features = {\n      conneg: config.conneg,\n      notifications: config.notifications,\n      // ...\n    };\n    this.pod = this.extractPodFromRequest(request);\n  }\n}\n```\n\n---\n\n### 20. Consolidate Logging\n**Priority**: MEDIUM\n\nReplace mixed `console.log/warn/error` with Fastify logger:\n\n```javascript\n// Before\nconsole.error('Config error:', err);\n\n// After\nfastify.log.error({ err }, 'Config error');\n```\n\n---\n\n## 📋 IMPLEMENTATION CHECKLIST\n\n### Phase 1: Security Fixes (BLOCKER for v0.1)\n- [ ] Fix path traversal in git handler\n- [ ] Implement or warn on agent group authorization  \n- [ ] Fix WebSocket subscription cleanup\n- [ ] Use safeJsonParse() consistently (5 files)\n- [ ] Add global security headers\n\n### Phase 2: Error Handling (HIGH for v0.1)\n- [ ] Create standardized error response helper\n- [ ] Update all handlers to use standard format\n- [ ] Replace silent catches with proper error returns\n- [ ] Add request correlation IDs\n\n### Phase 3: Code Quality (MEDIUM)\n- [ ] Extract shared utilities (getRequestPaths, etc.)\n- [ ] Create constants.js for magic numbers\n- [ ] Remove dead code (legacy DATA_ROOT export)\n- [ ] Add JSDoc to undocumented functions\n\n### Phase 4: Testing (MEDIUM)\n- [ ] Add git handler tests\n- [ ] Add integration tests for auth + WAC flow\n- [ ] Add range request tests\n- [ ] Add concurrent modification tests\n\n### Phase 5: Documentation (MEDIUM)\n- [ ] Document .pods endpoint\n- [ ] Document rate limiting behavior\n- [ ] Document WebSocket protocol\n- [ ] Add API reference for IdP endpoints\n\n### Phase 6: Performance (LOW - can defer)\n- [ ] Add ACL lookup caching\n- [ ] Cache etag calculations\n- [ ] Profile and optimize hot paths\n\n### Phase 7: Architecture (v0.2)\n- [ ] Abstract storage interface\n- [ ] Create request context objects\n- [ ] Refactor createServer() into smaller functions\n- [ ] Add observability/metrics framework\n\n---\n\n## EFFORT ESTIMATES\n\n| Phase | Difficulty | Days | Priority |\n|-------|------------|------|----------|\n| Phase 1: Security | 35/100 | 2-3 | P0 BLOCKER |\n| Phase 2: Error Handling | 25/100 | 2 | P0 |\n| Phase 3: Code Quality | 20/100 | 1-2 | P1 |\n| Phase 4: Testing | 30/100 | 2-3 | P1 |\n| Phase 5: Documentation | 15/100 | 1 | P1 |\n| Phase 6: Performance | 25/100 | 1-2 | P2 |\n| Phase 7: Architecture | 50/100 | 3-5 | v0.2 |\n\n**Total for v0.1**: ~10-13 days of focused work\n\n---\n\n## DEPENDENCIES\n\nThis issue relates to:\n- #99 - Docker (needs stable codebase)\n- #97 - Presets (independent)\n- #98 - CLI Wizard (independent)\n- #95 - Web Onboarding (needs stable codebase)\n- #96 - Admin Panel (needs stable codebase)\n\n**Recommended order**:\n1. This issue (v0.1 readiness) - Phases 1-5\n2. #99 Docker support\n3. #97 Presets\n4. #98 CLI Wizard\n5. #95 Web Onboarding\n6. #96 Admin Panel\n\n---\n\n## SUCCESS CRITERIA FOR v0.1\n\n- [ ] All CRITICAL and HIGH issues resolved\n- [ ] No known security vulnerabilities\n- [ ] Consistent error response format\n- [ ] Request tracing via correlation IDs\n- [ ] Core functionality has test coverage\n- [ ] API behaviors documented\n- [ ] Clean `npm audit` (0 vulnerabilities)\n- [ ] Docker image published to ghcr.io\n\n---\n\n## References\n\n- Security Audit (2026-01-05) - Internal\n- [OWASP Node.js Security Cheat Sheet](https://cheatsheetseries.owasp.org/cheatsheets/Nodejs_Security_Cheat_Sheet.html)\n- [Fastify Best Practices](https://fastify.dev/docs/latest/Guides/Recommendations/)\n- [Node.js Error Handling](https://nodejs.org/en/docs/guides/error-handling/)","author":{"url":"https://github.com/melvincarvalho","@type":"Person","name":"melvincarvalho"},"datePublished":"2026-01-19T13:33:13.000Z","interactionStatistic":{"@type":"InteractionCounter","interactionType":"https://schema.org/CommentAction","userInteractionCount":0},"url":"https://github.com/100/JavaScriptSolidServer/issues/100"}