quality-debt: PR #152 review feedback (critical)

[marcusquinn]

Unactioned Review Feedback

Source PR: #152
File: general
Reviewers: coderabbit
Findings: 1
Max severity: critical

---

CRITICAL: coderabbit (coderabbitai[bot])

Actionable comments posted: 6

🤖 Fix all issues with AI agents

In @.agent/tools/api/better-auth.md:
- Around line 40-53: Replace the non-null assertions for environment variables
in the betterAuth config by first reading and validating them (e.g., const
GOOGLE_CLIENT_ID = process.env.GOOGLE_CLIENT_ID) and throwing or returning a
clear error if missing; update the socialProviders.google block in the auth
configuration to use those validated constants instead of
process.env.GOOGLE_CLIENT_ID! and process.env.GOOGLE_CLIENT_SECRET! so runtime
failures are prevented and errors show which variable is absent.

In @.agent/tools/api/drizzle.md:
- Around line 222-249: The seed.ts script's seed() currently calls
db.delete(posts) and db.delete(users) unconditionally which will wipe production
data; modify seed() (and its invocation) to require an explicit,
environment-gated opt-in before performing destructive deletes: check NODE_ENV
(or a new env var like ALLOW_DB_WIPE) and refuse to run deletes when NODE_ENV
=== "production" unless ALLOW_DB_WIPE === "true" (or a CLI flag) is set; add a
clear, logged confirmation prompt or fail-fast error if the safety gate isn't
satisfied, and ensure the script exits without deleting when the guard blocks
the operation.

In @.agent/tools/api/hono.md:
- Around line 127-133: The middleware registered via app.use("/api/admin/*",
...) only checks for existence of the Authorization header
(c.req.header("Authorization")) but never validates the token; update this
middleware to parse the header (e.g., check "Bearer " scheme), validate/verify
the token using your auth utility or JWT verifier, return 401 on missing or
invalid token, and attach the decoded user/claims to the context (e.g.,
c.req.user or c.set("user", ...)) before calling await next() so downstream
handlers can trust the authenticated identity.
- Around line 209-221: The upload handler at app.post("/api/upload") currently
trusts the parsed File and reads its arrayBuffer without checks; add security
guards by enforcing a MAX_FILE_SIZE constant and rejecting requests when
file.size exceeds it before calling file.arrayBuffer(), validate file.type
against an explicit whitelist of allowed MIME types and return a 400 when
mismatched, and sanitize file.name (remove/normalize path segments and
disallowed chars or generate a safe server-side filename) before using it;
ensure you perform these checks on the parsed "file" variable in the same
handler and return clear JSON error responses for size/type/name failures.

In @.agent/tools/api/vercel-ai-sdk.md:
- Around line 124-217: The assistant HTML is rendered with
dangerouslySetInnerHTML using marked.parse in AIChatSidebar (inside the
displayMessages.map), which allows XSS; fix by sanitizing the parsed HTML before
insertion (e.g., import DOMPurify and replace __html: marked.parse(part.text)
with __html: DOMPurify.sanitize(marked.parse(part.text))). Add DOMPurify (and
types) to deps and ensure only assistant message parts use the sanitized HTML
path in the mapping where dangerouslySetInnerHTML is used.

In @.agent/tools/ui/i18next.md:
- Line 8: Frontmatter currently sets the boolean key "bash: false" while the
document contains a shell validation script; update the frontmatter to "bash:
true" so the subagent can run the validation script programmatically (or
explicitly state in the frontmatter that the included script is for manual
execution) — locate and edit the "bash" frontmatter key in this file to enable
bash execution for the validation script.

🧹 Nitpick comments (6)

.agent/tools/ui/tailwind-css.md (1)

70-78: Validate numeric values before injecting into CSS.

The pattern directly interpolates ${width} into inline styles. If this pattern is used with user-controlled input, ensure the value is validated as a number to prevent CSS injection.

🔒 Safer pattern with validation

 // Define in context/provider
+const sanitizedWidth = typeof width === 'number' && width > 0 ? width : 384;
-<style>{`:root { --sidebar-width: ${width}px; }`}</style>
+<style>{`:root { --sidebar-width: ${sanitizedWidth}px; }`}</style>

 // Use in className
 <aside className="w-[var(--sidebar-width)]">

.agent/tools/api/hono.md (1)

141-147: Consider production-safe error logging.

The error handler logs errors with console.error(err), which could expose stack traces in production logs. Consider using a structured logger with appropriate log levels and sanitization.

.agent/tools/ui/i18next.md (1)

248-256: Validation script lacks error handling and documentation.

The script assumes jq is installed and that locale directories exist in the current path. For DevOps automation excellence, add prerequisite checks and proper error handling.

🔧 Hardened validation script

+#!/usr/bin/env bash
+set -euo pipefail
+
+# Check prerequisites
+command -v jq >/dev/null 2>&1 || { echo "Error: jq is required" >&2; exit 1; }
+
+# Configuration
+BASE_LOCALE="en"
+TARGET_LOCALES=("de" "es" "fr")
+NAMESPACE="common"
+
 # Check for missing keys across locales
-for locale in de es fr; do
+for locale in "${TARGET_LOCALES[@]}"; do
+  if [[ ! -f "$locale/$NAMESPACE.json" ]]; then
+    echo "Warning: $locale/$NAMESPACE.json not found, skipping" >&2
+    continue
+  fi
   echo "=== Missing in $locale ==="
-  diff <(jq -r 'paths | join(".")' en/common.json | sort) \
-       <(jq -r 'paths | join(".")' $locale/common.json | sort) \
+  diff <(jq -r 'paths | join(".")' "$BASE_LOCALE/$NAMESPACE.json" | sort) \
+       <(jq -r 'paths | join(".")' "$locale/$NAMESPACE.json" | sort) \
        | grep "^<" | sed 's/^< //' || true
 done

.agent/tools/api/better-auth.md (1)

134-148: Consider documenting password requirements.

The email/password sign-up example doesn't mention password validation or strength requirements. For security best practices, document recommended password policies.

Example addition:

// Add password strength validation
const passwordSchema = z.string()
  .min(8, "Password must be at least 8 characters")
  .regex(/[A-Z]/, "Password must contain uppercase letter")
  .regex(/[a-z]/, "Password must contain lowercase letter")
  .regex(/[0-9]/, "Password must contain number");

const handleSignUp = async (data: {
  email: string;
  password: string;
  name: string;
}) => {
  // Validate password strength
  const validation = passwordSchema.safeParse(data.password);
  if (!validation.success) {
    console.error(validation.error);
    return;
  }
  // ... rest of sign-up
};

.agent/tools/ui/react-context.md (2)

82-91: Cookie persistence pattern works correctly.

The document.cookie pattern is appropriate for client-side UI state persistence. For production use with sensitive data, consider adding security attributes (Secure; SameSite=Lax) to the cookie string, though for sidebar state this is not critical.

🔒 Optional: Add security attributes to cookies

For enhanced security posture (especially if pattern is copied for sensitive data):

-document.cookie = `${COOKIE_NAME}=${value}; path=/; max-age=${COOKIE_MAX_AGE}`;
+document.cookie = `${COOKIE_NAME}=${value}; path=/; max-age=${COOKIE_MAX_AGE}; SameSite=Lax`;

Note: Secure attribute requires HTTPS and would be added in production environments.

Also applies to: 170-181

---

93-104: Clever CSS variable injection pattern!

The inline <style> tag approach for injecting dynamic CSS variables is innovative and works well for most applications. For projects with strict Content Security Policy (CSP), you might need to use CSS-in-JS libraries or CSS modules as alternatives, but this is not a concern for typical setups.

Also applies to: 189-196

View comment

---

Auto-generated by quality-feedback-helper.sh scan-merged. Review each finding and either fix the code or dismiss with a reason.

[github-actions]

Completed via PR #3506. merged to main.