fix: address Copilot review feedback

- writeEnvFile now quotes values containing whitespace/special chars
  and escapes embedded " and \. Fixes the round-trip corruption
  Copilot flagged: a pre-existing `FOO="a b"` would previously come
  back as `FOO=a b` after any unrelated config update because
  stripQuotes read but writeEnvFile didn't re-quote.
- fetchSkills no longer attaches the admin token — /api/skills is
  unauthenticated and there's no reason to broaden the token's
  exposure surface (server logs, future proxies) until the endpoint
  actually guards.
- POST /api/config wraps req.json() in try/catch and returns 400 for
  malformed JSON instead of leaking a 500 on a public endpoint.
- Comment on the isSafeEnvValue guard now reflects the full denylist
  (not just newlines) so future readers don't narrow the check by
  mistake.

Author: us

agent-templates/next/app/(agent)/_components/settings-panel.tsx

@@ -485,9 +485,10 @@ export default function SettingsPanel({ config, onChange }: { config: AgentConfi
 
   const fetchSkills = useCallback(async () => {
     try {
-      // Skills endpoint isn't auth-guarded today, but sending the header
-      // costs nothing and keeps us consistent if it ever becomes guarded.
-      const res = await fetch("/api/skills", { headers: configHeaders(false) });
+      // /api/skills is unauthenticated; keep the admin token out of its
+      // request surface (server logs, future proxies) until the endpoint
+      // actually requires it.
+      const res = await fetch("/api/skills");
       if (res.ok) {
         const data: SkillInfo[] = await res.json();
         setSkills(data.filter((s) => s.category !== "Export"));

agent-templates/next/app/(agent)/_lib/config/keys.ts

@@ -164,8 +164,18 @@ function readEnvFile(): Record<string, string> {
   }
 }
 
+// readEnvFile strips surrounding quotes, so we lose the knowledge that a
+// user's pre-existing value was originally quoted. Re-quote on write
+// whenever the value contains a character that dotenv would otherwise
+// mis-parse, escaping backslash and double-quote inside the quoted form.
+// Managed keys (filtered by isSafeEnvValue) never hit this branch; it
+// exists to preserve unrelated entries like FOO="a b" across round-trip.
 function writeEnvFile(vars: Record<string, string>) {
-  const lines = Object.entries(vars).map(([k, v]) => `${k}=${v}`);
+  const lines = Object.entries(vars).map(([k, v]) => {
+    if (!/[\s"'#$\\]/.test(v)) return `${k}=${v}`;
+    const escaped = v.replace(/\\/g, "\\\\").replace(/"/g, '\\"');
+    return `${k}="${escaped}"`;
+  });
   fs.writeFileSync(ENV_FILE, lines.join("\n") + "\n", "utf-8");
 }
 

agent-templates/next/app/(agent)/api/config/route.ts

@@ -27,7 +27,13 @@ export async function POST(req: Request) {
   const denied = requireConfigWriteAuth(req);
   if (denied) return denied;
 
-  const { keys, values } = (await req.json()) as {
+  let body: unknown;
+  try {
+    body = await req.json();
+  } catch {
+    return Response.json({ error: "Request body is not valid JSON" }, { status: 400 });
+  }
+  const { keys, values } = (body ?? {}) as {
     keys?: Record<string, string>;
     values?: Record<string, string>;
   };
@@ -36,8 +42,10 @@ export async function POST(req: Request) {
     return Response.json({ error: "Invalid request body" }, { status: 400 });
   }
 
-  // Reject payloads where any value would inject newlines into .env.local.
-  // Empty string means "delete this key" and is legal.
+  // Reject payloads whose values would break out of their .env.local line
+  // or trip dotenv's quote/comment/variable-expansion parsing (see
+  // isSafeEnvValue for the full denylist). Empty string means "delete
+  // this key" and is legal.
   for (const value of Object.values(keys ?? {})) {
     if (value !== "" && !isSafeEnvValue(value)) {
       return Response.json({ error: "Invalid characters in key value" }, { status: 400 });