| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
This 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.
Current Version: 0.0.81
Target Version: 0.1.0
Overall Readiness: ~65%
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Security | 1 | 1 | 5 | 0 |
| Error Handling | 0 | 1 | 3 | 1 |
| Code Quality | 0 | 0 | 5 | 4 |
| Performance | 0 | 1 | 2 | 2 |
| Testing | 0 | 0 | 3 | 1 |
| Documentation | 0 | 0 | 3 | 1 |
| Architecture | 0 | 0 | 3 | 1 |
| Total | 1 | 3 | 24 | 10 |
File: src/handlers/git.js lines 56-61
Severity: CRITICAL
Status: UNFIXED
// VULNERABLE: Uses path.resolve without urlToPath sanitization
const repoAbs = resolve(dataRoot, repoRelative);Issue: Git handler bypasses path traversal protection used elsewhere. An attacker could potentially access files outside the data directory.
Fix: Use urlToPath() from /src/utils/url.js which includes traversal protection:
import { urlToPath } from '../utils/url.js';
const repoAbs = urlToPath(repoRelative);File: src/wac/checker.js line 182
Severity: CRITICAL
Status: UNFIXED
// TODO: Check agent groupsIssue: WAC agentGroup authorization is silently ignored. Users expecting group-based access control will have security issues.
Options:
File: src/notifications/websocket.js lines 28-48
Severity: HIGH
Status: UNFIXED
const subscriptions = new Map(); // WebSocket -> Set<url>
const subscribers = new Map(); // url -> Set<WebSocket>Issue: No cleanup when WebSocket closes unexpectedly. Maps grow unbounded if close events don't fire properly.
Fix: Add explicit cleanup in close handler and periodic garbage collection:
socket.on('close', () => {
const urls = subscriptions.get(socket);
if (urls) {
for (const url of urls) {
subscribers.get(url)?.delete(socket);
if (subscribers.get(url)?.size === 0) {
subscribers.delete(url);
}
}
subscriptions.delete(socket);
}
});Files: Multiple (5 locations)
Severity: HIGH
Status: PARTIAL
safeJsonParse() exists in src/utils/json.js but not used consistently:
| File | Line | Status |
|---|---|---|
| src/wac/parser.js | 40 | ❌ Raw JSON.parse |
| src/auth/did-nostr.js | 129, 141 | ❌ Raw JSON.parse |
| src/nostr/relay.js | 242 | ❌ Raw JSON.parse |
| src/auth/token.js | 105 | ❌ Raw JSON.parse |
| src/ap/routes/inbox.js | 126 | ❌ Raw JSON.parse |
Fix: Replace all JSON.parse() with safeJsonParse() that has size limits and try/catch.
File: src/storage/filesystem.js lines 34, 49, 50+
Severity: MEDIUM
export async function read(urlPath) {
try {
return await fs.readFile(filePath);
} catch {
return null; // Can't distinguish "not found" from "permission error"
}
}Issue: Callers cannot distinguish between:
Fix: Return error objects or throw typed errors:
export async function read(urlPath) {
try {
return { data: await fs.readFile(filePath) };
} catch (err) {
if (err.code === 'ENOENT') return { error: 'not_found' };
if (err.code === 'EACCES') return { error: 'permission_denied' };
return { error: 'system_error', details: err.message };
}
}Files: Multiple handlers
Severity: MEDIUM
Current formats vary:
// Format A - Minimal
{ error: 'Not Found' }
// Format B - With message
{ error: 'Forbidden', message: 'Dotfile access is not allowed' }
// Format C - Extended
{ error: 'Too Many Requests', message: '...', retryAfter: 60 }Fix: Standardize all error responses:
{
error: 'ErrorCode',
message: 'Human readable description',
statusCode: 404,
details: { /* optional context */ }
}Create src/utils/errors.js:
export function errorResponse(reply, code, error, message, details = {}) {
return reply.code(code).send({
error,
message,
statusCode: code,
...details
});
}File: src/server.js
Severity: MEDIUM
Security headers only applied to mashlib responses, not globally.
Fix: Add global onSend hook:
fastify.addHook('onSend', async (request, reply) => {
reply.header('X-Content-Type-Options', 'nosniff');
reply.header('X-Frame-Options', 'DENY');
reply.header('X-XSS-Protection', '1; mode=block');
reply.header('Referrer-Policy', 'strict-origin-when-cross-origin');
});Files: All handlers
Severity: MEDIUM
No request IDs for tracing requests through logs.
Fix: Add request ID generation:
fastify.addHook('onRequest', async (request) => {
request.id = request.headers['x-request-id'] || crypto.randomUUID();
request.log = request.log.child({ requestId: request.id });
});
fastify.addHook('onSend', async (request, reply) => {
reply.header('X-Request-ID', request.id);
});Files: src/handlers/resource.js:24-31, src/handlers/container.js:15-22
Severity: LOW
Nearly identical implementations.
Fix: Extract to src/utils/paths.js:
export function getRequestPaths(request, dataRoot) {
const urlPath = decodeURIComponent(request.url.split('?')[0]);
const fullPath = urlToPath(urlPath);
return { urlPath, fullPath };
}Files: Multiple
Severity: LOW
Create src/constants.js:
export const LIMITS = {
SLUG_MAX_LENGTH: 255,
BODY_MAX_SIZE: 10 * 1024 * 1024, // 10MB
WS_MAX_SUBSCRIPTIONS: 100,
WS_MAX_URL_LENGTH: 2048,
};
export const CACHE = {
JTI_CLEANUP_INTERVAL: 60 * 1000,
JTI_TTL: 15 * 60 * 1000,
DPOP_MAX_AGE: 5 * 60,
};
export const RATE_LIMITS = {
GLOBAL_PER_MINUTE: 100,
WRITE_PER_MINUTE: 60,
POD_CREATION_PER_DAY: 1,
};Severity: MEDIUM
| Function | File | Lines | Issue |
|---|---|---|---|
| createServer() | src/server.js | 540 | God function, too many responsibilities |
| handleGet() | src/handlers/resource.js | 150+ | Multiple concerns mixed |
| authorize() | src/auth/middleware.js | 80+ | Complex nested conditionals |
Recommendation: Break into smaller focused functions. Example for createServer():
export async function createServer(options) {
const fastify = createFastifyInstance(options);
configureRequestContext(fastify, options);
registerSecurityPlugins(fastify, options);
registerFeaturePlugins(fastify, options);
registerRoutes(fastify, options);
return fastify;
}File: src/utils/url.js line 11
export let DATA_ROOT = './data'; // Legacy, superseded by getDataRoot()Fix: Remove legacy export, use only getDataRoot() function.
File: src/wac/checker.js lines 74-98
Severity: LOW (Performance)
while (currentStoragePath && currentStoragePath !== '/') {
// Linear walk up hierarchy for every request - no caching
}Fix: Add LRU cache for parsed ACL rules:
import { LRUCache } from 'lru-cache';
const aclCache = new LRUCache({ max: 500, ttl: 5 * 60 * 1000 });
async function getAcl(path) {
const cached = aclCache.get(path);
if (cached) return cached;
const acl = await loadAndParseAcl(path);
aclCache.set(path, acl);
return acl;
}Priority: MEDIUM
| File | Status |
|---|---|
| src/storage/filesystem.js | ❌ Most functions undocumented |
| src/handlers/resource.js | ⚠️ Partial |
| src/wac/parser.js | ❌ Complex ACL parsing undocumented |
| src/ap/routes/inbox.js | ❌ ActivityPub flow undocumented |
| src/auth/nostr.js | ❌ Schnorr verification undocumented |
Priority: MEDIUM
Document in README or API docs:
Priority: MEDIUM
| Area | Status | Priority |
|---|---|---|
| Git handler (src/handlers/git.js) | ❌ No tests | HIGH |
| Range requests (streaming) | ❌ No tests | MEDIUM |
| ActivityPub federation | ❌ Limited | MEDIUM |
| Nostr relay stress tests | ❌ None | LOW |
| Concurrent request handling | ❌ None | MEDIUM |
| WebID-TLS auth | ❌ No tests | LOW |
Priority: MEDIUM
Priority: LOW (v0.2)
Currently hardcoded to filesystem. Abstract for future backends:
// src/storage/interface.js
export interface StorageAdapter {
read(path: string): Promise<Buffer | null>;
write(path: string, data: Buffer): Promise<void>;
delete(path: string): Promise<void>;
exists(path: string): Promise<boolean>;
stat(path: string): Promise<Stats>;
list(path: string): Promise<string[]>;
}
// src/storage/filesystem.js
export class FilesystemAdapter implements StorageAdapter { ... }
// Future: src/storage/s3.js, src/storage/sqlite.jsPriority: LOW (v0.2)
Current approach decorates Fastify request with 15+ properties. Create proper context:
// Instead of request.connegEnabled, request.mashlibEnabled, etc.
class RequestContext {
constructor(request, config) {
this.config = config;
this.features = {
conneg: config.conneg,
notifications: config.notifications,
// ...
};
this.pod = this.extractPodFromRequest(request);
}
}Priority: MEDIUM
Replace mixed console.log/warn/error with Fastify logger:
// Before
console.error('Config error:', err);
// After
fastify.log.error({ err }, 'Config error');| Phase | Difficulty | Days | Priority |
|---|---|---|---|
| Phase 1: Security | 35/100 | 2-3 | P0 BLOCKER |
| Phase 2: Error Handling | 25/100 | 2 | P0 |
| Phase 3: Code Quality | 20/100 | 1-2 | P1 |
| Phase 4: Testing | 30/100 | 2-3 | P1 |
| Phase 5: Documentation | 15/100 | 1 | P1 |
| Phase 6: Performance | 25/100 | 1-2 | P2 |
| Phase 7: Architecture | 50/100 | 3-5 | v0.2 |
Total for v0.1: ~10-13 days of focused work
This issue relates to:
Recommended order:
| Back | FazBrowse Home | New Git URL |
Activity