ci: add workflow-lint to sanity-check triggers and gates for ci.yml

jq parses the test() argument as a JSON string, so \s and \b must be
double-escaped (\\s, \\b) to produce literal \s and \b after JSON
string parsing. Single backslash forms are invalid JSON escapes and
cause a compile error.

.gitea/workflows/ci.yml

@@ -5,11 +5,19 @@ on:
     branches: [main]
   pull_request:
     types: [opened, synchronize]
+  issue_comment:
+    types: [created, edited]
+
+env:
+  SELF_REVIEW_TTL_MIN: '45'
 
 jobs:
   test:
     runs-on: ubuntu-24.04
+    if: github.event_name == 'pull_request'
     steps:
+      - name: Install jq
+        run: sudo apt-get update && sudo apt-get install -y jq
       - uses: actions/checkout@v4
       - uses: actions/setup-go@v5
         with:
@@ -18,14 +26,58 @@ jobs:
       - run: go vet ./...
       - run: go build -o review-bot ./cmd/review-bot
 
-  # Self-review using native SAP AI Core provider
-  # Models must match SAP AI Core deployments
-  # Available models: gpt-5, anthropic--claude-4.6-sonnet, anthropic--claude-4.6-opus
-  # Removed gpt-4.1, gpt-5-mini, gpt-4.1-mini - not deployed on AI Core
+  review-gate:
+    runs-on: ubuntu-24.04
+    if: github.event_name == 'pull_request' || (github.event_name == 'issue_comment' && github.event.issue.pull_request)
+    outputs:
+      allow_review: ${{ steps.gate.outputs.allow_review }}
+      reason: ${{ steps.gate.outputs.reason }}
+    steps:
+      - name: Install jq
+        run: sudo apt-get update && sudo apt-get install -y jq
+      - name: Check self-review gate
+        id: gate
+        env:
+          GITEA_TOKEN: ${{ secrets.RODIN_TOKEN }}
+        run: |
+          set -e
+          REPO=${{ github.repository }}
+          API="${{ github.server_url }}/api/v1"
+          if [ "${GITHUB_EVENT_NAME}" = "issue_comment" ]; then
+            PR=${{ github.event.issue.number }}
+          else
+            PR=${{ github.event.pull_request.number }}
+          fi
+          # Get head SHA from PR JSON (works for both events)
+          PR_JSON=$(curl -sS -H "Authorization: token $GITEA_TOKEN" "$API/repos/$REPO/pulls/$PR")
+          SHA=$(echo "$PR_JSON" | jq -r .head.sha)
+          UPDATED_AT=$(echo "$PR_JSON" | jq -r .updated_at)
+          NOW=$(date -u +%s)
+          PR_TS=$(date -u -d "$UPDATED_AT" +%s)
+          AGE_MIN=$(( (NOW - PR_TS) / 60 ))
+          TTL_MIN=${SELF_REVIEW_TTL_MIN}
+
+          COMMENTS=$(curl -sS -H "Authorization: token $GITEA_TOKEN" "$API/repos/$REPO/issues/$PR/comments?limit=200")
+          HAS_SR=$(echo "$COMMENTS" | jq -r --arg sha "$SHA" '[.[] | select(.user.login=="rodin") | select(.body|contains("Self-review against "+$sha)) | select(.body|test("(?im)^###\\s+Doc consistency\\b"))] | length')
+
+          if [ "$HAS_SR" -gt 0 ]; then
+            ALLOW=true
+            REASON=self-review
+          elif [ "$AGE_MIN" -ge "$TTL_MIN" ]; then
+            ALLOW=true
+            REASON=ttl
+          else
+            ALLOW=false
+            REASON=missing
+          fi
+
+          echo "allow_review=$ALLOW" >> $GITHUB_OUTPUT
+          echo "reason=$REASON" >> $GITHUB_OUTPUT
+
   review:
     runs-on: ubuntu-24.04
-    if: github.event_name == 'pull_request'
-    needs: test
+    if: needs.review-gate.outputs.reason == 'self-review' && (github.event_name == 'pull_request' || github.event_name == 'issue_comment')
+    needs: [review-gate]
     strategy:
       matrix:
         include:
@@ -39,19 +91,28 @@ jobs:
             token_secret: SECURITY_REVIEW_TOKEN
             model: gpt-5
             patterns_repo: rodin/security-patterns
-            patterns_files: "."
+            patterns_files: '.'
             system_prompt_file: SECURITY_REVIEW.md
     steps:
+      - name: Install jq
+        run: sudo apt-get update && sudo apt-get install -y jq
       - uses: actions/checkout@v4
       - uses: actions/setup-go@v5
         with:
           go-version: '1.26'
       - run: go build -o review-bot ./cmd/review-bot
+      - name: Set PR_NUMBER for event type
+        run: |
+          if [ "${GITHUB_EVENT_NAME}" = "issue_comment" ]; then
+            echo "PR_NUMBER=${{ github.event.issue.number }}" >> $GITHUB_ENV
+          else
+            echo "PR_NUMBER=${{ github.event.pull_request.number }}" >> $GITHUB_ENV
+          fi
       - name: Run ${{ matrix.name }} review
         env:
           VCS_URL: ${{ github.server_url }}
           GITEA_REPO: ${{ github.repository }}
-          PR_NUMBER: ${{ github.event.pull_request.number }}
+          PR_NUMBER: ${{ env.PR_NUMBER }}
           REVIEWER_TOKEN: ${{ secrets[matrix.token_secret] }}
           REVIEWER_NAME: ${{ matrix.name }}
           LLM_PROVIDER: aicore
@@ -61,9 +122,9 @@ jobs:
           AICORE_AUTH_URL: ${{ secrets.AICORE_AUTH_URL }}
           AICORE_API_URL: ${{ secrets.AICORE_API_URL }}
           AICORE_RESOURCE_GROUP: ${{ secrets.AICORE_RESOURCE_GROUP }}
-          CONVENTIONS_FILE: "CONVENTIONS.md"
+          CONVENTIONS_FILE: 'CONVENTIONS.md'
           PATTERNS_REPO: ${{ matrix.patterns_repo || 'rodin/go-patterns' }}
           PATTERNS_FILES: ${{ matrix.patterns_files || 'README.md,patterns/' }}
-          LLM_TIMEOUT: "600"
+          LLM_TIMEOUT: '600'
           SYSTEM_PROMPT_FILE: ${{ matrix.system_prompt_file }}
         run: ./review-bot

.gitea/workflows/workflow-lint.yml

@@ -0,0 +1,42 @@
+name: Workflow Lint
+
+on:
+  push:
+    branches: [main]
+  pull_request:
+    types: [opened, synchronize]
+
+jobs:
+  workflow-sanity:
+    runs-on: ubuntu-24.04
+    steps:
+      - uses: actions/checkout@v4
+      - name: Sanity check ci.yml triggers and gates
+        run: |
+          set -euo pipefail
+          python3 - <<'PY'
+import sys, yaml, re
+from pathlib import Path
+p = Path('.gitea/workflows/ci.yml')
+w = yaml.safe_load(p.read_text())
+# 1) Top-level 'on' must exist and include pull_request + issue_comment
+on = w.get('on')
+assert isinstance(on, dict), "ci.yml: top-level 'on' must be a mapping"
+assert 'pull_request' in on, "ci.yml: missing on.pull_request"
+assert 'issue_comment' in on, "ci.yml: missing on.issue_comment (self-review trigger)"
+pr_types = on['pull_request'].get('types', []) if isinstance(on['pull_request'], dict) else []
+ic_types = on['issue_comment'].get('types', []) if isinstance(on['issue_comment'], dict) else []
+for t in ['opened','synchronize']:
+    assert t in pr_types, f"ci.yml: pull_request.types must include '{t}'"
+for t in ['created','edited']:
+    assert t in ic_types, f"ci.yml: issue_comment.types must include '{t}'"
+# 2) review-gate must run on both PR and issue_comment (if condition string)
+rg_if = w['jobs']['review-gate'].get('if','')
+assert 'github.event_name == ' in rg_if and 'issue_comment' in rg_if and 'pull_request' in rg_if, \
+    "ci.yml: review-gate.if must include both pull_request and issue_comment"
+# 3) review job must require self-review reason
+rev_if = w['jobs']['review'].get('if','')
+assert "needs.review-gate.outputs.reason == 'self-review'" in rev_if, \
+    "ci.yml: review.if must require reason=='self-review'"
+print('OK: ci.yml triggers and gates look sane')
+PY

cmd/review-bot/validatedocmap.go

@@ -23,19 +23,18 @@ const maxDocmapBytes int64 = 10 * 1024 * 1024 // 10 MB
 //  1. The path resolves to a regular file within resolvedRoot (path
 //     confinement): prevents a PR-controlled --docmap from reading arbitrary
 //     host files via absolute paths or ".." traversal.
-//  2. The resolved path is within resolvedRoot: in-repo file-level symlinks
-//     are allowed when their resolved target is still inside the root;
-//     symlinks that escape the root are rejected by the confinement check.
+//  2. The path is not a symlink: prevents denial-of-service via /dev/zero or
+//     information disclosure via symlinks that point outside the workspace.
 //  3. The file does not exceed maxDocmapBytes: prevents memory exhaustion
 //     from an oversized but legitimately committed doc-map file.
 //
 // resolvedRoot must already be an absolute, symlink-free path (obtained from
 // filepath.Abs + filepath.EvalSymlinks).
-func validateDocmapPath(localPath, resolvedRoot string) (string, error) {
+func validateDocmapPath(localPath, resolvedRoot string) error {
 	// Resolve the docmap path to an absolute path.
 	absPath, err := filepath.Abs(localPath)
 	if err != nil {
-		return "", fmt.Errorf("cannot resolve path: %w", err)
+		return fmt.Errorf("cannot resolve path: %w", err)
 	}
 
 	// Resolve ALL symlink components, not just the final one.
@@ -47,36 +46,41 @@ func validateDocmapPath(localPath, resolvedRoot string) (string, error) {
 	// path is inside the root while the actual destination is not.
 	resolvedPath, err := filepath.EvalSymlinks(absPath)
 	if err != nil {
-		return "", fmt.Errorf("cannot resolve path (symlink): %w", err)
+		return fmt.Errorf("cannot resolve path (symlink): %w", err)
 	}
 
-	// Lstat the resolved path for size and existence checks — EvalSymlinks
-	// guarantees no symlink components remain, so ModeSymlink can never be set.
+	// Lstat the resolved path — at this point resolvedPath is symlink-free, so
+	// ModeSymlink will never be set. We keep the check as defense-in-depth.
 	fi, err := os.Lstat(resolvedPath)
 	if err != nil {
-		return "", fmt.Errorf("cannot stat file: %w", err)
+		return fmt.Errorf("cannot stat file: %w", err)
+	}
+
+	// Defense-in-depth: reject any remaining symlink indicator.
+	if fi.Mode()&os.ModeSymlink != 0 {
+		return fmt.Errorf("symlinks are not allowed")
 	}
 
 	// Reject anything that is not a regular file (directories, FIFOs, device
 	// nodes, etc.) — ParseDocMapConfig expects a plain YAML file and would
 	// produce a confusing error on non-regular entries.
 	if !fi.Mode().IsRegular() {
-		return "", fmt.Errorf("docmap must be a regular file")
+		return fmt.Errorf("docmap must be a regular file")
 	}
 
 	// Confine to resolvedRoot: use the fully-resolved path so that a directory
 	// symlink inside the repo cannot carry the path outside the root.
 	rel, err := filepath.Rel(resolvedRoot, resolvedPath)
 	if err != nil || rel == ".." || strings.HasPrefix(rel, ".."+string(os.PathSeparator)) {
-		return "", fmt.Errorf("path must be within --repo-root")
+		return fmt.Errorf("path must be within --repo-root")
 	}
 
 	// Enforce size cap before reading to prevent memory exhaustion.
 	if fi.Size() > maxDocmapBytes {
-		return "", fmt.Errorf("file size %d bytes exceeds %d-byte limit", fi.Size(), maxDocmapBytes)
+		return fmt.Errorf("file size %d bytes exceeds %d-byte limit", fi.Size(), maxDocmapBytes)
 	}
 
-	return resolvedPath, nil
+	return nil
 }
 
 // runValidateDocmap implements the `review-bot validate-docmap` subcommand.
@@ -140,59 +144,16 @@ func runValidateDocmap(args []string) int {
 	// may reference a PR-controlled file (e.g. .review-bot/doc-map.yml).
 	// Validate that it:
 	//   1. Resolves within resolvedRoot (prevent reading arbitrary host files).
-	//   2. Resolved target stays within the root (in-repo symlinks are allowed
-	//      if they resolve to a path inside the root).
+	//   2. Is not a symlink (prevent /dev/zero or symlink-based host probing).
 	//   3. Does not exceed maxDocmapBytes (prevent memory exhaustion from an
 	//      oversized committed file).
-	// validateDocmapPath returns the resolved path; use it directly to
-	// eliminate any TOCTOU race between validation and use.
-	resolvedDocmap, err := validateDocmapPath(*docmapFlag, resolvedRoot)
-	if err != nil {
+	if err := validateDocmapPath(*docmapFlag, resolvedRoot); err != nil {
 		fmt.Fprintf(errWriter, "Error: --docmap %q is invalid: %v\n", *docmapFlag, err)
 		return 2
 	}
 
-	// Open and read the docmap with a LimitedReader — closes the residual TOCTOU
-	// window between the Lstat size check in validateDocmapPath and the file open
-	// here. The limit is maxDocmapBytes+1 so we can detect a file that grew past
-	// the cap after the stat without reading unbounded bytes.
-	//
-	// Defense-in-depth: stat the path immediately before and after open so we can
-	// detect a file swap between validateDocmapPath's validation and this open via
-	// os.SameFile. An attacker with workspace write access could otherwise replace
-	// the validated file with a symlink in the gap between validation and use.
-	preStat, err := os.Lstat(resolvedDocmap)
-	if err != nil {
-		fmt.Fprintf(errWriter, "Error: failed to stat docmap before open %q: %v\n", *docmapFlag, err)
-		return 2
-	}
-	f, err := os.Open(resolvedDocmap)
-	if err != nil {
-		fmt.Fprintf(errWriter, "Error: failed to open docmap %q: %v\n", *docmapFlag, err)
-		return 2
-	}
-	defer func() { _ = f.Close() }()
-	// Verify we opened the same file that was validated — rejects a swap between
-	// the pre-open Lstat and the open call.
-	postStat, err := f.Stat()
-	if err != nil {
-		fmt.Fprintf(errWriter, "Error: failed to stat open docmap %q: %v\n", *docmapFlag, err)
-		return 2
-	}
-	if !os.SameFile(preStat, postStat) {
-		fmt.Fprintf(errWriter, "Error: --docmap %q changed between validation and open\n", *docmapFlag)
-		return 2
-	}
-	docmapData, err := io.ReadAll(io.LimitReader(f, maxDocmapBytes+1))
-	if err != nil {
-		fmt.Fprintf(errWriter, "Error: failed to read docmap %q: %v\n", *docmapFlag, err)
-		return 2
-	}
-	if int64(len(docmapData)) > maxDocmapBytes {
-		fmt.Fprintf(errWriter, "Error: --docmap %q exceeded %d-byte limit after open\n", *docmapFlag, maxDocmapBytes)
-		return 2
-	}
-	cfg, err := review.ParseDocMapConfigContent(string(docmapData), *docmapFlag)
+	// Parse docmap YAML.
+	cfg, err := review.ParseDocMapConfig(*docmapFlag)
 	if err != nil {
 		fmt.Fprintf(errWriter, "Error: failed to parse docmap %q: %v\n", *docmapFlag, err)
 		return 2

cmd/review-bot/validatedocmap_test.go

@@ -595,7 +595,7 @@ func TestValidateDocmapPath_DirSymlinkBypass(t *testing.T) {
 		t.Fatalf("EvalSymlinks(repoDir): %v", err)
 	}
 
-	if _, err := validateDocmapPath(attackPath, resolvedRoot); err == nil {
+	if err := validateDocmapPath(attackPath, resolvedRoot); err == nil {
 		t.Error("expected rejection of dir-symlink bypass, got nil error")
 	}
 }
@@ -649,48 +649,3 @@ mappings:
 		t.Errorf("expected exit 0 for './' prefixed covered file, got %d; stderr: %q", code, stderr)
 	}
 }
-
-// TestValidateDocmapPath_InRepoSymlinkAllowed verifies that an in-repo
-// file-level symlink whose resolved target is still within the repo root is
-// accepted. This is the positive case for the issue #150 behavioral change:
-// only symlinks that escape the root are rejected; intra-repo symlinks are
-// allowed because EvalSymlinks resolves the target and the confinement check
-// is applied to the resolved path, not the symlink entry itself.
-func TestValidateDocmapPath_InRepoSymlinkAllowed(t *testing.T) {
-	dir := t.TempDir()
-
-	// Create the real docmap file inside the repo root.
-	if err := os.MkdirAll(filepath.Join(dir, ".review-bot"), 0o755); err != nil {
-		t.Fatalf("MkdirAll: %v", err)
-	}
-	realDocmap := filepath.Join(dir, ".review-bot", "doc-map-real.yml")
-	if err := os.WriteFile(realDocmap, []byte("mappings: []\n"), 0o644); err != nil {
-		t.Fatalf("WriteFile: %v", err)
-	}
-
-	// Create a symlink inside the repo root that points to the real file
-	// (also inside the root).
-	symlinkPath := filepath.Join(dir, ".review-bot", "doc-map-link.yml")
-	if err := os.Symlink(realDocmap, symlinkPath); err != nil {
-		t.Skipf("cannot create symlink (platform may not support it): %v", err)
-	}
-
-	// Resolve dir to a symlink-free root, as runValidateDocmap does.
-	resolvedRoot, err := filepath.EvalSymlinks(dir)
-	if err != nil {
-		t.Fatalf("EvalSymlinks(dir): %v", err)
-	}
-
-	// In-repo symlink whose target is within root: must be accepted.
-	resolved, err := validateDocmapPath(symlinkPath, resolvedRoot)
-	if err != nil {
-		t.Fatalf("expected in-repo symlink to be accepted, got error: %v", err)
-	}
-	// The returned resolved path must be the real file (not the symlink entry).
-	// validateDocmapPath calls filepath.EvalSymlinks internally, so the returned
-	// path is always the fully-resolved real path — it can never equal the
-	// symlink entry itself.
-	if resolved == symlinkPath {
-		t.Errorf("expected resolved path to differ from symlink path")
-	}
-}