-
Notifications
You must be signed in to change notification settings - Fork 0
Harden packaged runtime startup #299
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 4 commits
ffac562
e4fae43
bea1586
b2724d6
d744949
e70ab8f
463ed71
339ec84
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -170,14 +170,23 @@ async function runPrefetch(installed: InstalledAppEntry): Promise<void> { | |
| const rawCwd = prefetch.cwd ? substituteTemplate(prefetch.cwd, vars) : packageRoot; | ||
| const cwd = isAbsolute(rawCwd) ? rawCwd : resolvePath(packageRoot, rawCwd); | ||
| const command = resolveCommand(prefetch.command, packageRoot); | ||
| const args = substituteArgs(prefetch.args, vars); | ||
| if (!canSpawnResolvedCommand(command)) { | ||
| console.log(`[AAP] Prefetch skipped: ${manifest.id}`, { | ||
| command: prefetch.command, | ||
| reason: "command unavailable", | ||
| }); | ||
| return; | ||
| } | ||
| const args = substituteArgs(prefetch.args, vars); | ||
| const missingEntrypoint = findMissingPrefetchEntrypoint(args, cwd); | ||
| if (missingEntrypoint) { | ||
| console.log(`[AAP] Prefetch skipped: ${manifest.id}`, { | ||
| command: prefetch.command, | ||
| reason: "entrypoint unavailable", | ||
| entrypoint: missingEntrypoint, | ||
| }); | ||
| return; | ||
| } | ||
| const env = createBackendChildEnv({ | ||
| ...substituteEnv(prefetch.env, vars), | ||
| DEUS_APP_ID: manifest.id, | ||
|
|
@@ -233,6 +242,16 @@ async function runPrefetch(installed: InstalledAppEntry): Promise<void> { | |
| }); | ||
| } | ||
|
|
||
| function findMissingPrefetchEntrypoint(args: string[], cwd: string): string | null { | ||
| const [firstArg] = args; | ||
| if (!firstArg) return null; | ||
| if (firstArg.startsWith("-")) return null; | ||
| if (!firstArg.includes("/") && !firstArg.includes("\\")) return null; | ||
|
|
||
| const entrypoint = isAbsolute(firstArg) ? firstArg : resolvePath(cwd, firstArg); | ||
| return existsSync(entrypoint) ? null : entrypoint; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The new missing-entrypoint guard treats any first arg containing Useful? React with 👍 / 👎.
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in d744949: the prefetch entrypoint guard now ignores URI-like first operands such as https://... and file:..., so valid URL operands are not treated as missing filesystem paths. Added an integration case that runs a prefetch command with an https:// first arg. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Treating any first arg containing Useful? React with 👍 / 👎.
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in e70ab8f. The prefetch missing-entrypoint guard now only treats explicit filesystem entrypoints as paths (absolute paths, ./, ../, Windows drive paths), so scoped package arguments like |
||
| } | ||
|
|
||
| function canSpawnResolvedCommand(command: string): boolean { | ||
| if (isAbsolute(command) || command.includes("/") || command.includes("\\")) { | ||
| return existsSync(command); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
runPrefetchnow callssubstituteArgsbefore checkingcanSpawnResolvedCommand, so a manifest with templated prefetch args (for example{workspace}) will throw immediately whenvarsis empty even in the “command unavailable” path that is supposed to be skipped. BecauseprefetchInstalledAppAssets()fire-and-forgetsrunPrefetch, this rejection is unhandled and can crash startup under Node’s default unhandled-rejection behavior instead of logging a benign skip.Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in d744949: prefetch now checks whether the resolved command is spawnable before expanding cwd/args/env templates, so unavailable optional prefetch commands skip cleanly instead of rejecting on empty boot-time vars. Added integration coverage for a missing prefetch command with templated cwd/args.