fix: narrow launch-safety private path checks#8
Conversation
|
Warning Review limit reached
More reviews will be available in 51 minutes and 35 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (37)
📝 WalkthroughWalkthroughPR introduces optional environment-variable-driven private export and refresh status configuration, refactors build and refresh automation away from hardcoded Windows paths, updates documentation, and performs a uniform timestamp refresh across all pages and manifests to 4 Jun 2026. ChangesPrivate Export and Refresh Infrastructure
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/verify-launch-safety.js (1)
162-173:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winCritical: Service worker cache date is stale.
The pipeline is failing because
CACHE_NAMEinservice-worker.jsis set tostackscout-2026-06-04, but the visible issue date inindex.htmlis2026-06-06. The validation logic at line 171 correctly enforces that the cache name must not be older than the published issue date.According to the review stack context, timestamps were refreshed to 4 Jun 2026 in layer 5, but the actual
index.htmlcontent now shows 6 Jun 2026 (today's date). This indicates either:
index.htmlwas updated after the timestamp refresh, or- The refresh script generated today's date instead of the intended 4 Jun date.
Action required: Update
CACHE_NAMEinservice-worker.jstostackscout-2026-06-06to match the issue date inindex.html, or regenerate all timestamps consistently.🐛 Proposed fix for service-worker.js
Update the CACHE_NAME constant:
-const CACHE_NAME = 'stackscout-2026-06-04' +const CACHE_NAME = 'stackscout-2026-06-06'Alternatively, verify the refresh script and regenerate all timestamps consistently if 4 Jun was the intended date.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/verify-launch-safety.js` around lines 162 - 173, The service worker cache date is stale: in assertServiceWorkerFreshness() the cache name extracted by extractCacheName() (CACHE_NAME in service-worker.js) is older than the visible issue date from extractIssueDate(); update CACHE_NAME in service-worker.js to "stackscout-2026-06-06" so cacheDateMatch[1] >= extractIssueDate(), or if 2026-06-04 was intended, regenerate timestamps so extractIssueDate() and CACHE_NAME are consistent (ensure changes reflect in service-worker.js and any refresh script).
🧹 Nitpick comments (1)
scripts/verify-launch-safety.js (1)
50-50: ⚡ Quick winConsider excluding generic localhost/127.0.0.1 from UNC pattern.
The estate UNC pattern includes
localhostand127.0.0.1, which are commonly used in generic examples, tutorials, and documentation. Including them may trigger false positives when the public site content quotes or references generic local development patterns.The PR objective states the goal is to "avoid false positives caused by public feed text quoting generic Windows paths." If
localhost/127.0.0.1references in public content are intentional examples (not leaks of actual private paths), they should be removed from this pattern.♻️ Proposed fix to exclude generic localhost references
- { label: 'estate UNC path', pattern: /\\\\(?:\?\\)?(?:nas_storage_1|MINI-PC|localhost|127\.0\.0\.1)[\\/][^\s"'<>)]*/i }, + { label: 'estate UNC path', pattern: /\\\\(?:\?\\)?(?:nas_storage_1|MINI-PC)[\\/][^\s"'<>)]*/i },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/verify-launch-safety.js` at line 50, Update the UNC detection regex for the entry with label 'estate UNC path' to remove the generic hosts "localhost" and "127.0.0.1" from the alternation so the pattern only matches specific machine names like "nas_storage_1" or "MINI-PC"; locate the object with label 'estate UNC path' and edit its pattern property to drop the localhost/127.0.0.1 alternatives while preserving the rest of the UNC matching (leading backslashes and path segment matching).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/verify-launch-safety.js`:
- Line 51: The regex in PRIVATE_PATTERNS for the "local-only surface marker" is
case-sensitive and misses the lowercase "local-only"; update the
PRIVATE_PATTERNS entry (the object with label 'local-only surface marker') to
either include the lowercase variant (e.g., add 'local-only' to the alternation)
or apply the case-insensitive /i flag so it matches all case forms, and then
remove or correct the accompanying comment/assertion about a CACHE_NAME vs
index.html "Updated …" date mismatch (referencing CACHE_NAME and the
'index.html' Updated line) so it reflects the actual checked-in filenames/dates
or drops the incorrect claim.
---
Outside diff comments:
In `@scripts/verify-launch-safety.js`:
- Around line 162-173: The service worker cache date is stale: in
assertServiceWorkerFreshness() the cache name extracted by extractCacheName()
(CACHE_NAME in service-worker.js) is older than the visible issue date from
extractIssueDate(); update CACHE_NAME in service-worker.js to
"stackscout-2026-06-06" so cacheDateMatch[1] >= extractIssueDate(), or if
2026-06-04 was intended, regenerate timestamps so extractIssueDate() and
CACHE_NAME are consistent (ensure changes reflect in service-worker.js and any
refresh script).
---
Nitpick comments:
In `@scripts/verify-launch-safety.js`:
- Line 50: Update the UNC detection regex for the entry with label 'estate UNC
path' to remove the generic hosts "localhost" and "127.0.0.1" from the
alternation so the pattern only matches specific machine names like
"nas_storage_1" or "MINI-PC"; locate the object with label 'estate UNC path' and
edit its pattern property to drop the localhost/127.0.0.1 alternatives while
preserving the rest of the UNC matching (leading backslashes and path segment
matching).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f399a950-fe1c-4dbc-98d9-9cc414028996
📒 Files selected for processing (42)
.env.example.gitignoreREADME.mdcatalog/index.htmlcategories/apis/index.htmlcategories/clis/index.htmlcategories/desktop-apps/index.htmlcategories/index.htmlcategories/mcps/index.htmlcategories/web-apps/index.htmlcategories/web-services/index.htmlcategories/websites/index.htmlcollections/index.htmldata/categories-manifest.jsondata/collections-manifest.jsondata/methodology-manifest.jsondata/page-registry.jsondata/radar-manifest.jsondata/tools-manifest.jsondata/updates-manifest.jsonindex.htmlmethod/index.htmlradar/index.htmlscripts/build-stackscout.jsscripts/refresh-stackscout.ps1scripts/verify-launch-safety.jsservice-worker.jstools/anthropic-api/index.htmltools/canvas-planner/index.htmltools/gh-cli/index.htmltools/github-mcp-server/index.htmltools/hugging-face/index.htmltools/make/index.htmltools/n8n/index.htmltools/openai-api/index.htmltools/perplexity/index.htmltools/playwright-mcp/index.htmltools/signal-stack/index.htmltools/strudel-studio/index.htmltools/syncpad/index.htmltools/wrangler/index.htmlupdates/index.html
| { label: 'Kol Windows user path', pattern: /\b[A-Z]:[\\/]Users[\\/](?:koltregaskes|kolin)[\\/][^\s"'<>)]*/i }, | ||
| { label: 'W drive estate path', pattern: /\bW:[\\/][^\s"'<>)]*/i }, | ||
| { label: 'estate UNC path', pattern: /\\\\(?:\?\\)?(?:nas_storage_1|MINI-PC|localhost|127\.0\.0\.1)[\\/][^\s"'<>)]*/i }, | ||
| { label: 'local-only surface marker', pattern: /\b(?:tools-hub-local|LOCAL-ONLY|_local)\b/ }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Search for lowercase 'local-only' in public output that might be missed by the case-sensitive pattern
rg -n --type html --type js --type json -g '!verify-launch-safety.js' '\blocal-only\b'Repository: koltregaskes/stackscout
Length of output: 49
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Inspect the exact PRIVATE_PATTERNS entries + where assertNoPrivateLeaks applies them.
sed -n '1,120p' scripts/verify-launch-safety.js | cat -n
sed -n '120,220p' scripts/verify-launch-safety.js | cat -n
# 2) Repo-wide occurrences of the marker variants (not just html/js/json)
rg -n --hidden --no-ignore-vcs --glob '!.git/*' '\b(local-only|LOCAL-ONLY|_local)\b' . || true
# 3) Also check for just "tools-hub-local" and "LOCAL-ONLY" as plain strings (case-sensitive)
rg -n --hidden --no-ignore-vcs --glob '!.git/*' 'tools-hub-local|LOCAL-ONLY|_local' . || trueRepository: koltregaskes/stackscout
Length of output: 8923
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Extract CACHE_NAME and index.html "Updated" date (what assertServiceWorkerFreshness compares)
echo "== service-worker.js CACHE_NAME =="
rg -n "const CACHE_NAME" service-worker.js
cache_name="$(rg -o "const CACHE_NAME = ['\"]([^'\"]+)['\"]" service-worker.js | sed -E "s/.*='([^']+)'.*/\1/" | head -n1)"
echo "CACHE_NAME=$cache_name"
echo
echo "== index.html Updated date string =="
updated_line="$(rg -n "Updated ([0-9]{1,2}) (Jan|Feb|Mar|Apr|May|Jun|Jul|Aug|Sep|Oct|Nov|Dec) ([0-9]{4})" index.html | head -n1 || true)"
echo "$updated_line"
# Parse into YYYY-MM-DD using same mapping as script (quick inline)
python3 - <<'PY'
import re, pathlib
text = pathlib.Path("index.html").read_text(encoding="utf-8", errors="ignore")
m = re.search(r"Updated\s+([0-9]{1,2})\s+(Jan|Feb|Mar|Apr|May|Jun|Jul|Aug|Sep|Oct|Nov|Dec)\s+([0-9]{4})", text)
month_map={"Jan":"01","Feb":"02","Mar":"03","Apr":"04","May":"05","Jun":"06","Jul":"07","Aug":"08","Sep":"09","Oct":"10","Nov":"11","Dec":"12"}
if not m:
print("ISSUE_DATE=auto-today (no match)")
else:
day=int(m.group(1))
issue=f"{m.group(3)}-{month_map[m.group(2)]}-{day:02d}"
print("ISSUE_DATE="+issue)
PY
# 2) Search only in paths that are scanned by collectPublicTextFiles():
# PUBLIC_ENTRIES = index.html, catalog, categories, collections, data, method, radar, tools, updates, app.js, pwa.js, service-worker.js, manifest.json, styles.css, sitemap.xml, icon.svg
# TEXT_EXTENSIONS = .css .html .js .json .svg .txt .xml
echo
echo "== Markers in scanned public text files (local-only / LOCAL-ONLY / tools-hub-local / _local) =="
rg -n --hidden --no-ignore-vcs \
-g '*.css' -g '*.html' -g '*.js' -g '*.json' -g '*.svg' -g '*.txt' -g '*.xml' \
-e '\b(local-only|LOCAL-ONLY|tools-hub-local|_local)\b' \
index.html catalog categories collections data method radar tools updates \
app.js pwa.js service-worker.js manifest.json styles.css sitemap.xml icon.svg || trueRepository: koltregaskes/stackscout
Length of output: 438
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n --hidden --no-ignore-vcs --glob '!.git/*' \
'\b(local-only|LOCAL-ONLY|tools-hub-local|_local)\b' . \
|| trueRepository: koltregaskes/stackscout
Length of output: 743
Fix case-sensitive local-only marker detection in PRIVATE_PATTERNS
scripts/verify-launch-safety.js uses a case-sensitive regex for the “local-only surface marker” (/\b(?:tools-hub-local|LOCAL-ONLY|_local)\b/), so it will not match lowercase local-only. Current public output doesn’t contain these marker strings, but this gap could allow a local-only leak through if it ever appears. Include the lowercase variant (or add an /i flag) to make detection consistent.
Also, the CACHE_NAME vs index.html “Updated …” date mismatch claim doesn’t apply to the checked-in files here (stackscout-2026-06-04 vs Updated 4 Jun 2026 → 2026-06-04).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/verify-launch-safety.js` at line 51, The regex in PRIVATE_PATTERNS
for the "local-only surface marker" is case-sensitive and misses the lowercase
"local-only"; update the PRIVATE_PATTERNS entry (the object with label
'local-only surface marker') to either include the lowercase variant (e.g., add
'local-only' to the alternation) or apply the case-insensitive /i flag so it
matches all case forms, and then remove or correct the accompanying
comment/assertion about a CACHE_NAME vs index.html "Updated …" date mismatch
(referencing CACHE_NAME and the 'index.html' Updated line) so it reflects the
actual checked-in filenames/dates or drops the incorrect claim.
|
Closing in favour of clean replacement branch fix/stackscout-launch-validation-clean. The original branch picked up noisy generated-data history during repair. |
Summary
Verification
Notes
Summary by CodeRabbit
Documentation
Chores
Refactor