fix(security): close approval-gate spoofing + path traversal (#54) — v1.7.1

CHANGELOG.md

@@ -5,6 +5,20 @@
 All notable changes to RStack are documented here. Entries are user-focused:
 what you can now do that you couldn't before.
 
+## [1.7.1] - 2026-06-02
+
+### Security
+- **Approval gates can no longer be spoofed or abused** (closes a pre-release
+  audit). Approving from the dashboard now requires a signed token
+  (`RSTACK_APPROVAL_TOKEN`) and a same-origin request, and records audit-proof
+  actor evidence — a script can't submit an arbitrary manager name anymore.
+  Without the token configured, browser approvals are disabled by default;
+  `sdlc_approve` continues to enforce the manager allow-list.
+- **Approval ids can no longer escape the run directory.** Run ids, task ids,
+  and artifact names in approval ids are strictly validated and every write is
+  asserted to stay inside `.rstack/runs/<run>` with a real manifest — closing a
+  path-traversal write.
+
 ## [1.7.0] - 2026-06-02
 
 ### Added

README.md

@@ -67,6 +67,10 @@ Switch project/run scope from the top bar; share `#run=<id>` links in Slack.
 - The moment a gate blocks, **every configured channel is paged** and the
   dashboard pops a notification
 - Every approval records the real approver (git identity or `RSTACK_USER`)
+- Approving from the dashboard requires a signed token (`RSTACK_APPROVAL_TOKEN`)
+  so a manager's identity can't be spoofed from the browser; `sdlc_approve`
+  enforces the `policy.json` manager allow-list. Without the token set,
+  browser approvals are disabled (secure default) — approve via `sdlc_approve`.
 
 ## Notifications — Slack, Teams, Discord, Telegram, WhatsApp
 
@@ -112,6 +116,8 @@ instrumentation is on the roadmap.
 | `RSTACK_USER` / `RSTACK_USER_EMAIL` | Identity for runs/approvals (defaults to git config) |
 | `RSTACK_BUSINESS_PORT` | Dashboard port (default 3008) |
 | `RSTACK_NO_BUSINESS_HUB=1` | Disable dashboard auto-launch |
+| `RSTACK_APPROVAL_TOKEN` | Required to approve from the dashboard (prevents identity spoofing) |
+| `RSTACK_MANAGER_USERS` | Comma-separated manager allow-list (also in `policy.json`) |
 | `RSTACK_SLACK_WEBHOOK` etc. | Notification channels — see webhooks guide |
 | `RSTACK_DEFAULT_MODEL` / `RSTACK_ESCALATED_MODEL` | Models for delegated builders (escalation at attempt ≥ 2) |
 

package.json

@@ -1,6 +1,6 @@
 {
   "name": "rstack-agents",
-  "version": "1.7.0",
+  "version": "1.7.1",
   "description": "Production-ready agentic SDLC framework for Pi and coding agents — orchestrator, builder/validator teams, lifecycle state, and specialist reuse",
   "type": "module",
   "main": "src/index.js",

src/core/tracker/approvals.js

@@ -1,17 +1,40 @@
 import { readFile, writeFile, mkdir } from 'node:fs/promises';
 import { existsSync } from 'node:fs';
-import { join } from 'node:path';
+import { join, resolve, sep } from 'node:path';
 
 // owner: RStack developed by Richardson Gunde
 
 const QUEUE_FILE = '.rstack/approvals.jsonl';
 
-function queuePath(projectRoot) {
-  return join(projectRoot, QUEUE_FILE);
+// Run ids are timestamp-slug strings — never path separators or traversal.
+// A crafted approval id could otherwise encode a runId like "../../etc" and
+// drive a write outside .rstack/runs (issue #54). Validate before any FS use.
+const SAFE_RUN_ID = /^[A-Za-z0-9][A-Za-z0-9._-]{0,200}$/;
+
+export function isSafeRunId(runId) {
+  return typeof runId === 'string' && SAFE_RUN_ID.test(runId) && !runId.includes('..');
+}
+
+function safeArtifact(artifact) {
+  // Artifacts are file/stage names, not paths.
+  return typeof artifact === 'string' && artifact.length > 0 && artifact.length < 256
+    && !artifact.includes('/') && !artifact.includes('\\') && !artifact.includes('..');
 }
 
-function runApprovalsPath(projectRoot, runId) {
-  return join(projectRoot, '.rstack', 'runs', runId, 'approvals.json');
+// Resolve a run's approvals.json and assert it stays inside .rstack/runs/<runId>.
+// Returns null if the runId is unsafe, escapes the sandbox, or the run has no
+// manifest.json (i.e. it isn't a real run).
+function safeRunApprovalsPath(projectRoot, runId) {
+  if (!isSafeRunId(runId)) return null;
+  const runsRoot = resolve(projectRoot, '.rstack', 'runs');
+  const runDir = resolve(runsRoot, runId);
+  if (runDir !== join(runsRoot, runId) || !(runDir === runsRoot || runDir.startsWith(runsRoot + sep))) return null;
+  if (!existsSync(join(runDir, 'manifest.json'))) return null;
+  return join(runDir, 'approvals.json');
+}
+
+function queuePath(projectRoot) {
+  return join(projectRoot, QUEUE_FILE);
 }
 
 function policyPath(projectRoot) {
@@ -34,7 +57,11 @@ export function parseApprovalQueueId(id) {
   if (typeof id !== 'string' || !id.startsWith('gate:')) return null;
   const [, runId, taskId, artifact] = id.split(':');
   if (!runId || !artifact) return null;
-  return { runId: decodePart(runId), taskId: decodePart(taskId), artifact: decodePart(artifact) };
+  const decoded = { runId: decodePart(runId), taskId: decodePart(taskId), artifact: decodePart(artifact) };
+  // Reject ids whose decoded parts could traverse the filesystem.
+  if (!isSafeRunId(decoded.runId) || !safeArtifact(decoded.artifact)) return null;
+  if (decoded.taskId && (decoded.taskId.includes('/') || decoded.taskId.includes('..'))) return null;
+  return decoded;
 }
 
 async function readJson(path, fallback) {
@@ -111,8 +138,9 @@ export async function readApprovals(projectRoot) {
 
 export async function appendRunApproval(projectRoot, runId, record) {
   if (!runId || !record?.artifact) return null;
-  const path = runApprovalsPath(projectRoot, runId);
-  await mkdir(join(projectRoot, '.rstack', 'runs', runId), { recursive: true });
+  // Hard sandbox: unsafe/escaping runId, or a run with no manifest, writes nothing.
+  const path = safeRunApprovalsPath(projectRoot, runId);
+  if (!path) return null;
   const approvals = await readJson(path, []);
   const all = Array.isArray(approvals) ? approvals : [];
   const next = {
@@ -134,7 +162,10 @@ export async function resolveApproval(projectRoot, id, decision, resolvedBy, opt
   const idx = all.findIndex(a => a.id === id);
   const parsed = idx === -1 ? parseApprovalQueueId(id) : null;
   if (idx === -1 && !parsed) return false;
-  if (idx === -1 && parsed && !existsSync(join(projectRoot, '.rstack', 'runs', parsed.runId))) return false;
+  // parseApprovalQueueId already rejected unsafe runIds; require a real run.
+  if (idx === -1 && parsed && !safeRunApprovalsPath(projectRoot, parsed.runId)) return false;
+  // A queued entry could predate validation — re-check before trusting its runId.
+  if (idx !== -1 && all[idx].runId && !isSafeRunId(all[idx].runId)) return false;
 
   const base = idx === -1 ? {
     id,
@@ -152,7 +183,9 @@ export async function resolveApproval(projectRoot, id, decision, resolvedBy, opt
   const queueStatus = decision === 'approved' ? 'approved' : 'rejected';
   const runStatus = decision === 'approved' ? 'APPROVED' : 'REJECTED';
   const resolvedAt = new Date().toISOString();
-  const next = { ...base, status: queueStatus, resolvedBy: approver, resolvedAt, updatedAt: resolvedAt };
+  // Audit-proof actor evidence, not just a name string.
+  const actor = options.actor ? { ...options.actor } : { name: approver, via: 'api', tokenVerified: false, ts: resolvedAt };
+  const next = { ...base, status: queueStatus, resolvedBy: approver, actor, resolvedAt, updatedAt: resolvedAt };
 
   if (idx === -1) all.push(next);
   else all[idx] = next;

src/observability/dashboard/server.js

@@ -114,34 +114,66 @@ async function broadcastSnapshot() {
   broadcast(toClientState(state));
 }
 
+// A signed approval is required whenever RSTACK_APPROVAL_TOKEN is set: the
+// dashboard cannot mint manager identity from an unauthenticated request body.
+// Without the env token, approving from the browser is blocked entirely (the
+// secure default for a multi-user company hub) — set the token to enable it.
+function approvalAuthError(req) {
+  const expected = process.env.RSTACK_APPROVAL_TOKEN;
+  if (!expected) {
+    return { code: 403, msg: 'dashboard approvals are disabled — set RSTACK_APPROVAL_TOKEN to enable signed approvals, or approve via sdlc_approve' };
+  }
+  // CSRF: a cross-site form POST cannot set custom headers and would carry a
+  // foreign Origin. Require the token header and a localhost (or absent) origin.
+  const origin = req.headers.origin;
+  if (origin && !/^https?:\/\/(localhost|127\.0\.0\.1)(:\d+)?$/.test(origin)) {
+    return { code: 403, msg: 'cross-origin approval rejected' };
+  }
+  const token = req.headers['x-rstack-approval-token'];
+  if (!token || token !== expected) {
+    return { code: 401, msg: 'missing or invalid approval token' };
+  }
+  return null;
+}
+
 async function handleApproval(req, res, decision) {
-  let body = '';
-  req.on('error', () => {
+  const fail = (code, msg) => {
     if (!res.headersSent) {
-      res.writeHead(400, { 'Content-Type': 'application/json' });
-      res.end(JSON.stringify({ ok: false, error: 'request stream error' }));
+      res.writeHead(code, { 'Content-Type': 'application/json' });
+      res.end(JSON.stringify({ ok: false, error: msg }));
     }
+  };
+  const contentType = String(req.headers['content-type'] ?? '');
+  if (!contentType.includes('application/json')) {
+    return fail(415, 'Content-Type must be application/json');
+  }
+  const authErr = approvalAuthError(req);
+  if (authErr) return fail(authErr.code, authErr.msg);
+
+  let body = '';
+  let tooLarge = false;
+  req.on('error', () => fail(400, 'request stream error'));
+  req.on('data', (chunk) => {
+    body += chunk;
+    if (body.length > 64 * 1024) { tooLarge = true; req.destroy(); }
   });
-  req.on('data', (chunk) => { body += chunk; });
   req.on('end', async () => {
+    if (tooLarge) return fail(413, 'request body too large');
     try {
-      const { id, resolvedBy } = safeJson(body) ?? {};
-      if (!id) {
-        res.writeHead(400, { 'Content-Type': 'application/json' });
-        res.end(JSON.stringify({ ok: false, error: 'missing approval id' }));
-        return;
-      }
-      const ok = await resolveDashboardApproval(PROJECT_ROOT, id, decision, resolvedBy ?? 'dashboard');
+      const parsed = safeJson(body) ?? {};
+      const { id, resolvedBy } = parsed;
+      if (!id) return fail(400, 'missing approval id');
+      if (!resolvedBy || typeof resolvedBy !== 'string') return fail(400, 'resolvedBy (approver identity) is required');
+      // Actor evidence: token-verified, not just a body string.
+      const ok = await resolveDashboardApproval(PROJECT_ROOT, id, decision, resolvedBy, {
+        actor: { name: resolvedBy, via: 'dashboard', tokenVerified: true, ts: new Date().toISOString() },
+      });
       res.writeHead(ok ? 200 : 404, { 'Content-Type': 'application/json' });
       res.end(JSON.stringify({ ok }));
       if (ok) await broadcastSnapshot();
     } catch (err) {
       process.stderr.write(`[rstack-business] approval error: ${err?.message}\n`);
-      if (!res.headersSent) {
-        const statusCode = Number(err?.statusCode) || 500;
-        res.writeHead(statusCode, { 'Content-Type': 'application/json' });
-        res.end(JSON.stringify({ ok: false, error: String(err?.message) }));
-      }
+      fail(Number(err?.statusCode) || 500, String(err?.message));
     }
   });
 }

src/observability/dashboard/state/approvals.js

@@ -71,9 +71,9 @@ export function summarizeApprovals(queueApprovals) {
   };
 }
 
-export async function resolveApprovalAcrossRoots(roots, id, decision, resolvedBy) {
+export async function resolveApprovalAcrossRoots(roots, id, decision, resolvedBy, options = {}) {
   for (const root of roots ?? []) {
-    const ok = await resolveApproval(root, id, decision, resolvedBy);
+    const ok = await resolveApproval(root, id, decision, resolvedBy, options);
     if (ok) return true;
   }
   return false;

src/observability/dashboard/state/index.js

@@ -91,7 +91,7 @@ export async function buildFullState(projectRoot, options = {}) {
 
 export async function resolveDashboardApproval(projectRoot, id, decision, resolvedBy, options = {}) {
   const roots = await sourceRoots(projectRoot, options);
-  return resolveApprovalAcrossRoots(roots, id, decision, resolvedBy);
+  return resolveApprovalAcrossRoots(roots, id, decision, resolvedBy, { actor: options.actor });
 }
 
 function buildFrameworks(runs) {

src/observability/dashboard/ui/client.js

@@ -1278,9 +1278,16 @@ function resolveApproval(id, action) {
     resolvedBy = window.prompt('Manager name for this approval decision') || '';
     if (resolvedBy) localStorage.setItem('rstack-approver-name', resolvedBy);
   }
+  // Approvals require the signed token (RSTACK_APPROVAL_TOKEN) so identity
+  // can't be spoofed from a bare request. Stored locally after first entry.
+  var token = localStorage.getItem('rstack-approval-token') || '';
+  if (!token && typeof window.prompt === 'function') {
+    token = window.prompt('Approval token (RSTACK_APPROVAL_TOKEN set on the hub)') || '';
+    if (token) localStorage.setItem('rstack-approval-token', token);
+  }
   fetch('/api/' + action, {
     method: 'POST',
-    headers: { 'Content-Type': 'application/json' },
+    headers: { 'Content-Type': 'application/json', 'x-rstack-approval-token': token },
     body: JSON.stringify({ id: id, resolvedBy: resolvedBy || 'dashboard' })
   }).then(function(response) {
     if (!response.ok) {

tests/approval-security.test.js

@@ -0,0 +1,94 @@
+/**
+ * Security regression tests for the approval gate (issue #54):
+ *   1. approval-id path traversal cannot write outside .rstack/runs
+ *   2. manager identity / unsafe queue entries are rejected
+ *
+ * owner: RStack developed by Richardson Gunde
+ */
+
+import test from 'node:test';
+import assert from 'node:assert/strict';
+import { existsSync, mkdtempSync, mkdirSync, readdirSync, rmSync, writeFileSync } from 'node:fs';
+import { tmpdir } from 'node:os';
+import { join } from 'node:path';
+import {
+  isSafeRunId,
+  parseApprovalQueueId,
+  approvalQueueId,
+  appendRunApproval,
+  resolveApproval,
+} from '../src/core/tracker/approvals.js';
+
+function realRun(projectRoot, runId) {
+  mkdirSync(join(projectRoot, '.rstack', 'runs', runId), { recursive: true });
+  writeFileSync(join(projectRoot, '.rstack', 'runs', runId, 'manifest.json'), JSON.stringify({ run_id: runId }));
+}
+
+test('isSafeRunId rejects traversal and separators', () => {
+  assert.equal(isSafeRunId('2026-06-02T05-35-04-500Z-build-thing'), true);
+  assert.equal(isSafeRunId('../../etc'), false);
+  assert.equal(isSafeRunId('a/b'), false);
+  assert.equal(isSafeRunId('..'), false);
+  assert.equal(isSafeRunId(''), false);
+  assert.equal(isSafeRunId(null), false);
+});
+
+test('parseApprovalQueueId rejects ids that decode to a traversal runId', () => {
+  const evil = 'gate:' + encodeURIComponent('../../../../tmp/evil') + '::' + encodeURIComponent('plan.md');
+  assert.equal(parseApprovalQueueId(evil), null);
+  const evilArtifact = 'gate:' + encodeURIComponent('run-1') + '::' + encodeURIComponent('../../escape');
+  assert.equal(parseApprovalQueueId(evilArtifact), null);
+  // A legitimate id still round-trips.
+  const good = approvalQueueId({ runId: 'run-1', taskId: '004-impl', artifact: 'plan.md' });
+  assert.deepEqual(parseApprovalQueueId(good), { runId: 'run-1', taskId: '004-impl', artifact: 'plan.md' });
+});
+
+test('appendRunApproval refuses to write outside .rstack/runs', async () => {
+  const projectRoot = mkdtempSync(join(tmpdir(), 'rstack-sec-'));
+  const outside = mkdtempSync(join(tmpdir(), 'rstack-outside-'));
+  const traversal = '..' + '/'.repeat(1) + '..' + '/' + outside.split('/').pop();
+
+  const result = await appendRunApproval(projectRoot, traversal, { artifact: 'plan.md', status: 'APPROVED', approver: 'x' });
+  assert.equal(result, null, 'unsafe runId writes nothing');
+  // Nothing landed in the sibling temp dir.
+  assert.equal(existsSync(join(outside, 'approvals.json')), false);
+
+  // A real run still works.
+  realRun(projectRoot, 'good-run');
+  const ok = await appendRunApproval(projectRoot, 'good-run', { artifact: 'plan.md', status: 'APPROVED', approver: 'x' });
+  assert.ok(ok && existsSync(join(projectRoot, '.rstack', 'runs', 'good-run', 'approvals.json')));
+
+  rmSync(projectRoot, { recursive: true, force: true });
+  rmSync(outside, { recursive: true, force: true });
+});
+
+test('resolveApproval rejects a traversal gate id and requires a real run', async () => {
+  const projectRoot = mkdtempSync(join(tmpdir(), 'rstack-sec-'));
+  const evil = approvalQueueId({ runId: '../../../../tmp/pwn', taskId: 't', artifact: 'plan.md' });
+  assert.equal(await resolveApproval(projectRoot, evil, 'approved', 'Manager'), false, 'traversal id refused');
+
+  // Safe shape but no such run (no manifest) → refused, no file created.
+  const ghost = approvalQueueId({ runId: 'ghost-run', taskId: 't', artifact: 'plan.md' });
+  assert.equal(await resolveApproval(projectRoot, ghost, 'approved', 'Manager'), false);
+  assert.equal(existsSync(join(projectRoot, '.rstack', 'runs', 'ghost-run')), false);
+  rmSync(projectRoot, { recursive: true, force: true });
+});
+
+test('manager allow-list blocks names outside policy', async () => {
+  const projectRoot = mkdtempSync(join(tmpdir(), 'rstack-sec-'));
+  realRun(projectRoot, 'run-x');
+  mkdirSync(join(projectRoot, '.rstack'), { recursive: true });
+  writeFileSync(join(projectRoot, '.rstack', 'policy.json'), JSON.stringify({ managers: ['Maya'] }));
+  const id = approvalQueueId({ runId: 'run-x', taskId: 't', artifact: 'plan.md' });
+
+  await assert.rejects(
+    () => resolveApproval(projectRoot, id, 'approved', 'Imposter', { env: {} }),
+    /not allowed by manager policy/,
+  );
+  // The configured manager succeeds and records token-verified actor evidence.
+  const ok = await resolveApproval(projectRoot, id, 'approved', 'Maya', {
+    env: {}, skipRunWrite: true, actor: { name: 'Maya', via: 'dashboard', tokenVerified: true, ts: 'now' },
+  });
+  assert.equal(ok, true);
+  rmSync(projectRoot, { recursive: true, force: true });
+});