Skip to content

rewrite: quickslice backend + cloudflare pages frontend#72

Merged
zzstoatzz merged 6 commits intomainfrom
quickslice-rewrite
Dec 13, 2025
Merged

rewrite: quickslice backend + cloudflare pages frontend#72
zzstoatzz merged 6 commits intomainfrom
quickslice-rewrite

Conversation

@zzstoatzz
Copy link
Owner

summary

complete rewrite using quickslice as the backend instead of custom rust.

  • backend: quickslice on fly.io (oauth, graphql api, jetstream ingestion)
  • frontend: vanilla js spa on cloudflare pages

what's new

  • AT protocol oauth via quickslice
  • emoji picker with bufos
  • user preferences (display name visibility)
  • markdown link rendering in statuses
  • spa routing (feed page, settings)

deployments

notes

builds quickslice from source at v0.17.3 to include the sub claim fix for oauth token response.

🤖 Generated with Claude Code

zzstoatzz and others added 4 commits December 13, 2025 17:26
🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- build quickslice from source at v0.17.3 (includes sub claim fix)
- frontend on cloudflare pages, backend on fly.io
- add readme with deployment docs
- clean up old deployment artifacts

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@claude
Copy link

claude bot commented Dec 13, 2025

Pull Request Review: Quickslice Backend Rewrite

Overview

This is a substantial architectural rewrite replacing the custom Rust implementation with Quickslice (Gleam-based backend) + vanilla JS frontend. The changes are well-documented and the migration appears thoughtfully executed.


🟢 Strengths

Architecture & Design

  • Clean separation: Backend on Fly.io, frontend on Cloudflare Pages is a solid modern approach
  • Technology choice: Leveraging Quickslice eliminates custom OAuth/GraphQL implementation - good use of existing tools
  • Lexicon design: The io.zzstoatzz.status and io.zzstoatzz.status.preferences lexicons are simple and appropriate

Code Quality

  • Well-structured frontend: The SPA routing, state management, and UI components in app.js are organized logically
  • Good UX features: Emoji picker with Bufos integration, user preferences, markdown link rendering
  • Responsive design: CSS is clean with proper theming support and mobile considerations

🟡 Areas for Improvement

1. Security Concerns

Critical - Dockerfile line 18:

ENV GIT_TERMINAL_PROMPT=0

While this disables interactive prompts, the Dockerfile clones from a public repo without commit hash verification. Consider:

# Pin to specific commit for reproducibility and security
RUN git clone --depth 1 https://github.com/bigmoves/quickslice.git /build && \
    cd /build && \
    git fetch --depth 1 origin v0.17.3 && \
    git checkout v0.17.3 && \
    git verify-tag v0.17.3  # if tags are signed

Medium - GraphQL Query Injection (app.js:73-90):

query: `
  query GetPreferences($did: String!) {
    ioZzstoatzzStatusPreferences(
      where: { did: { eq: $did } }
      first: 1
    ) {

Good use of parameterized queries with variables! However, ensure Quickslice backend validates/sanitizes the did parameter server-side.

Low - External Script Loading (index.html:9):

<script src="https://cdn.jsdelivr.net/gh/bigmoves/quickslice@main/quickslice-client-js/dist/quickslice-client.min.js"></script>

Loading from @main branch is a security/stability risk. Pin to a specific version:

<script src="https://cdn.jsdelivr.net/gh/bigmoves/quickslice@v0.17.3/quickslice-client-js/dist/quickslice-client.min.js" integrity="sha384-..." crossorigin="anonymous"></script>

Add SRI (Subresource Integrity) hash for additional security.

2. Performance Considerations

app.js - Multiple GraphQL Queries:

  • Lines 321-382: loadUserHistory() and loadGlobalFeed() make separate queries
  • Consider implementing cursor-based pagination instead of offset (line 358: offset + limit)
  • The first: 100 limit in loadUserFrequentEmojis() (line 343) could be expensive for active users

Recommendation:

// Use cursor-based pagination for better performance
const loadMore = async () => {
  const lastCursor = feedData[feedData.length - 1]?.cursor;
  // Use 'after: $cursor' instead of offset
};

Emoji Data Loading:

  • Line 377: Fetches 15.1.0 emoji dataset from CDN every time - consider caching with service worker
  • bufos.json is 1614 lines - ensure it's properly compressed/gzipped

3. Code Quality Issues

app.js:16 - Hardcoded Configuration:

const CONFIG = {
  server: 'https://zzstoatzz-quickslice-status.fly.dev',
  clientId: 'client_2mP9AwgVHkg1vaSpcWSsKw',
};

These should be environment variables or build-time configuration for different environments (dev/staging/prod).

Error Handling Gaps:

  • Lines 96, 134: console.error() but user sees no feedback
  • Line 126: Silent deletion failure on old preferences
  • Consider user-facing error states and retry mechanisms

Magic Numbers:

  • Line 343: first: 100 - should be a named constant
  • Line 435: slice(0, 50) - should be MAX_EMOJI_SEARCH_RESULTS

4. Potential Bugs

Race Condition in Settings (app.js:107-135):
When saving preferences, old records are deleted then new one is created. If the create fails, preferences are lost. Use a transaction or update-or-insert pattern.

Timezone Handling:
The timestamp formatter doesn't account for user timezone preferences. All dates use the browser's local timezone which may be confusing.

Lexicon Validation (lexicons/status.json:16):

"maxLength": 64

For emoji field allows 64 chars, but custom emoji format custom:bufo-stab could be quite long. Consider if this is sufficient for all custom emoji slugs.

5. Deployment & Infrastructure

fly.toml:14 - Environment Variables:

DATABASE_URL = 'sqlite:/data/quickslice.db'

SQLite on a single machine is fine for personal use, but document scaling limitations in README.

Missing Health Checks:
No health check configuration in fly.toml - consider adding:

[http_service.checks]
  [http_service.checks.health]
    grace_period = "10s"
    interval = "30s"
    method = "GET"
    path = "/health"

Dockerfile Multi-stage Concerns:

  • Line 50: Runtime image includes curl - only needed for dbmate installation, could be removed after
  • No user specified - container runs as root (security risk)

6. Testing & Documentation

Missing:

  • No tests in the PR (unit, integration, or E2E)
  • No error boundary for the SPA
  • No CHANGELOG documenting breaking changes from previous version
  • Migration guide for existing users

📋 Recommendations Summary

High Priority

  1. Pin external dependencies (quickslice CDN script) to specific versions with SRI hashes
  2. Add error boundaries and user-facing error messages
  3. Implement transactional preference updates to avoid data loss
  4. Add health check endpoints

Medium Priority

  1. Extract hardcoded config to environment variables
  2. Implement cursor-based pagination for better performance
  3. Add Dockerfile security improvements (non-root user, minimal runtime dependencies)
  4. Add basic test coverage for critical paths (auth, status creation, preferences)

Low Priority

  1. Replace magic numbers with named constants
  2. Add service worker for emoji dataset caching
  3. Document scaling considerations
  4. Add migration guide for existing users

✅ Final Verdict

This is a well-executed rewrite that modernizes the architecture and removes significant custom code complexity. The frontend is clean and the Quickslice integration is appropriate.

Approval Status: Approved with suggestions

The security and performance concerns noted above are typical for a v1 rewrite and none are blocking. I recommend addressing the high-priority items (particularly dependency pinning and error handling) before or shortly after merging.

Great work on the comprehensive rewrite! The move to Quickslice should significantly reduce maintenance burden.


Review generated by Claude Code

@zzstoatzz zzstoatzz merged commit 6660acb into main Dec 13, 2025
1 check passed
@zzstoatzz zzstoatzz deleted the quickslice-rewrite branch December 13, 2025 23:57
@claude
Copy link

claude bot commented Dec 14, 2025

Pull Request Review

This PR represents a complete architectural rewrite from Rust to quickslice (Gleam/Erlang) backend with a vanilla JS SPA frontend. Overall execution is solid with modern patterns, but there are critical security issues that must be addressed before merge.


⚠️ CRITICAL Security Issues

1. XSS Vulnerability in parseLinks() (site/app.js:643-656)

Risk Level: HIGH

URLs are not validated before insertion, allowing javascript: protocol URLs that enable XSS attacks.

Attack example: [click me](javascript:alert(document.cookie))

Fix Required: Add URL scheme validation:

function isValidUrl(url) {
  try {
    const parsed = new URL(url);
    return ['http:', 'https:'].includes(parsed.protocol);
  } catch { return false; }
}

2. Unpinned CDN Dependency (site/index.html:9)

Using @main branch introduces supply chain risk. Pin to specific version/commit.

3. Unsafe Preferences Deletion (site/app.js:125-137)

Errors in deletion loop are silently swallowed, potentially creating duplicate records.


✅ Strengths

  • Clean architecture: Proper separation between backend (Fly.io) and frontend (Cloudflare Pages)
  • Modern stack: Good choice leveraging quickslice for AT Protocol
  • Security basics: Consistent use of escapeHtml() before rendering user content
  • Docker best practices: Multi-stage build, proper volume management
  • Good UX: Emoji picker, user preferences, markdown links, custom bufos

🧪 Test Coverage: MISSING

All test infrastructure removed (.github/workflows/ci.yml) with no replacement.

Required:

  • E2E tests for OAuth, status CRUD, emoji picker
  • GraphQL API contract tests
  • Basic smoke tests at minimum

🚀 Performance Issues

  1. Large emoji data: 1,614 lines in bufos.json loaded on every page - should lazy-load
  2. No caching: Missing Cache-Control headers for static assets
  3. Double image requests: onerror fallback .png→.gif causes unnecessary requests
  4. Multiple GraphQL round trips: Consider query batching

📋 Code Quality

Issues:

  • Using alert() for errors (poor UX - use toast notifications)
  • Hardcoded URLs scattered throughout
  • No bundler/TypeScript (1,220 lines of vanilla JS)
  • Global state management (client, userPreferences, emojiData)

Good patterns:

  • Template literals for HTML generation
  • Event delegation on dynamic content
  • Proper timestamp formatting utilities

🏗️ Infrastructure Concerns

  1. Missing CI/CD: No GitHub Actions workflows for deployment
  2. Docker: Git clone without checksum verification (line 20) - tag could be force-pushed
  3. Missing files: Dockerfile references patches/mist-websocket-protocol.patch not in repo
  4. Migration path: How do existing users migrate from Rust backend? Needs documentation

🔍 Lexicon Review

preferences.json:

  • ✅ Good: Uses literal:self key, enum for theme
  • ⚠️ Issue: font field allows 64-char string but only 4 values supported - should use enum

status.json:

  • ✅ Good: Properly expanded maxLength to support custom:name format

📝 Summary

BLOCKERS (must fix):

  1. XSS vulnerability - add URL validation in parseLinks()
  2. No tests - add minimum smoke/E2E tests
  3. ⚠️ Pin CDN dependency to specific version

Should fix soon:

  1. Add CI/CD workflows
  2. Document migration path
  3. Fix preferences deletion error handling
  4. Replace alert() with proper error UI

Nice to have:

  1. Lazy-load emoji data
  2. Add caching headers
  3. Font enum in lexicon
  4. SHA verification for Docker git clone

✅ Verdict

CONDITIONAL APPROVAL - Solid architectural rewrite with good design decisions, but XSS vulnerability and lack of tests are merge blockers. Once critical security issues are resolved and basic tests added, this is ready to ship.

Impressive work on this major rewrite! The new stack is well-chosen and implementation shows good understanding of modern web practices. 🚀

@claude
Copy link

claude bot commented Dec 14, 2025

Pull Request Review: Quickslice Rewrite

Overview

This is a significant architectural rewrite that moves from a custom Rust backend to the Quickslice framework (Gleam/Erlang) with a vanilla JavaScript SPA frontend. The PR successfully implements the core status app functionality with improved features.

Strengths

Architecture

  • Clean separation: Backend on Fly.io, frontend on Cloudflare Pages
  • Leveraging Quickslice reduces maintenance burden
  • Multi-stage Docker build with proper separation
  • Modern GraphQL API

Features

  • Enhanced user preferences (themes, fonts, accent colors)
  • Rich emoji support with intelligent picker
  • Frequent emoji tracking, markdown links, expiring statuses
  • Responsive, accessible UI

Critical Issues

1. XSS in Markdown Rendering (site/app.js:1067-1075)

Doesn't prevent javascript: or data: URIs.

Fix: Add protocol validation

if (!safeUrl.match(/^https?:\/\//i)) return match;

2. External Resources (site/app.js:531)

Loading bufo images without CSP.

  • Implement CSP headers
  • Consider proxying images

3. No Input Validation

Status text length not validated before submission.

  • Add maxLength: 128 validation

Performance Issues

  1. Full page reloads (lines 933, 952) - use state updates instead
  2. No debouncing on search (line 586) - add 300ms debounce
  3. Loading 1614 bufos at once - use virtual scrolling

Code Quality

  1. Monolithic 1220-line app.js - split into modules
  2. Magic numbers - use constants
  3. Inconsistent error handling - use toast system
  4. No tests - add coverage

Config Issues

  1. Duplicate memory config in fly.toml
  2. Missing docs for required secrets
  3. No health check endpoint
  4. Removed CI workflow

Recommendations Priority

High (Before Production)

  1. Fix XSS (protocol validation)
  2. CSP headers
  3. Input validation
  4. Document secrets
  5. Health checks

Medium

  1. Debouncing
  2. Error handling
  3. State updates
  4. Tests
  5. Split modules

Overall

Well-executed rewrite with modern architecture. Address security issues (XSS, validation) before production. The Quickslice migration is a good decision.

Recommendation: Approve with changes - fix critical security issues first.

Great work! 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant