Skip to content

fix: narrow server-entry evidence and pin standardize to the checked app repo (#4137) - #4249

Merged
atomantic merged 1 commit into
mainfrom
claim/issue-4137-followup
Aug 15, 2026
Merged

fix: narrow server-entry evidence and pin standardize to the checked app repo (#4137)#4249
atomantic merged 1 commit into
mainfrom
claim/issue-4137-followup

Conversation

@atomantic

Copy link
Copy Markdown
Owner

Summary

Follow-up to #4233 (which was rebase-merged while a final review round was still in flight, so this commit missed the merge). Two review findings against the app-type classification that #4233 introduced:

1. app.js / main.js were counted as server entry points. classifyNonNodeType treated server, app, and main.js/.mjs/.cjs/.ts) as evidence that "a process runs here", which suppresses the static classification. But beside an index.html, app.js and main.js are at least as likely to be browser scripts the page loads — so a real static site stayed unknown and was still offered the Node PM2 standardizer. That is the exact false positive #4137 was filed about, traded in to avoid a much rarer false negative. The basename list is now just server, the one name that can only mean a server (ecosystem.config.* still counts, unchanged).

2. /api/standardize/* could standardize a repo it never type-checked. resolveStandardizeTarget in server/routes/standardize.js returned repoPath || app.repoPath, so it ran standardizeRefusalFor(app) against the record named by appId and then rewrote whatever repoPath the request supplied. A permitted Node appId paired with another directory would carry a Python or Docker project straight past the refusal, making the gate decorative — and #4233 is what gave that gate real work to do. It now returns the checked record's own repoPath. No caller sends both (apiSystem.js's wrappers send one or the other, and the socket handler already used app.repoPath), so this only closes the hole.

Test plan

  • cd server && npx vitest run services/streamingDetect.test.js services/pm2Standardizer.test.js routes/standardize.test.js services/socket.test.js routes/apps — 272 passed / 1 skipped.
  • Updated classifyNonNodeType coverage: the server-basename list now asserts only server.{js,mjs,cjs,ts} and the ecosystem configs suppress classification, with a new case pinning index.html + index.js/app.js/main.js as static.
  • New route test: an appId sent alongside a foreign repoPath analyzes the app's own repo, not the supplied one.

Refs #4137

@atomantic
atomantic merged commit 60e678b into main Aug 15, 2026
7 checks passed
@atomantic
atomantic deleted the claim/issue-4137-followup branch August 15, 2026 03:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant