fix(lsp): address codex review round 3
Final review pass on the local provider: - Tear the instance down when `initialize` REJECTS (utf-8 negotiation, malformed result), not only on abort, so a permanently-rejecting `ready` is never pooled. - Use the group-aware SIGKILL on a framing failure so helpers are reached. - Validate maxMessageBytes and maxDocumentBytes positive at load alongside the other byte caps. - Fix the location renderer's outside-workspace check to match a `..` segment exactly, so an in-workspace path like `..generated/a.ts` stays relative. - Document the accepted ancestor-directory symlink-swap TOCTOU under the trusted-host model (O_NOFOLLOW guards only the final component).
This commit is contained in:
@@ -187,9 +187,10 @@ export class LspConnection {
|
||||
try {
|
||||
messages = this.decoder.push(chunk)
|
||||
} catch (error) {
|
||||
// A framing/JSON failure corrupts the stream position irrecoverably: fail the instance.
|
||||
// A framing/JSON failure corrupts the stream position irrecoverably: fail the instance and
|
||||
// SIGKILL the whole group so helper processes don't outlive the leader.
|
||||
this.fail(asError(error))
|
||||
this.child.kill('SIGKILL')
|
||||
this.signalGroup('SIGKILL')
|
||||
return
|
||||
}
|
||||
for (const message of messages) this.dispatch(message)
|
||||
|
||||
@@ -114,8 +114,12 @@ export function apply(ctx: Context, config: Config): void {
|
||||
// nonpositive value would let a server that ignores shutdown hang disposal forever. Fail at load.
|
||||
assertPositiveInteger('shutdownTimeoutMs', resolved.shutdownTimeoutMs)
|
||||
assertPositiveInteger('killGraceMs', resolved.killGraceMs)
|
||||
// A nonpositive stderr cap defeats the retained-tail bound (`slice(-0)` keeps everything).
|
||||
// Byte caps must be positive: a nonpositive stderr cap defeats the retained-tail bound
|
||||
// (`slice(-0)` keeps everything), `maxMessageBytes: 0` makes every response fatal, and a bad
|
||||
// document cap fails later in the read path instead of at load.
|
||||
assertPositiveInteger('maxStderrBytes', resolved.maxStderrBytes)
|
||||
assertPositiveInteger('maxMessageBytes', resolved.maxMessageBytes)
|
||||
assertPositiveInteger('maxDocumentBytes', resolved.maxDocumentBytes)
|
||||
const childEnv = buildChildEnv(resolved.env)
|
||||
// Resolve the executable eagerly so a misconfigured command fails at load, not on first query.
|
||||
const executable = resolveExecutable(resolved.command, childEnv)
|
||||
|
||||
@@ -108,16 +108,17 @@ export class LspInstance {
|
||||
if (this.disposed) throw new Error('LSP instance was disposed')
|
||||
/* v8 ignore next -- the abortable queue wait rejects a pre-aborted signal before runQuery; this is a belt-and-suspenders guard. */
|
||||
if (signal?.aborted) throw abortError(signal)
|
||||
// Observe abort during the handshake wait: a server that never answers `initialize` must not
|
||||
// block the tool-timeout signal here (the timeout policy awaits our quiescence, not the promise).
|
||||
// If abort wins, the handshake is still pending on a live process, so tear the instance down —
|
||||
// otherwise its poisoned `ready` would make every later query for this workspace re-wait.
|
||||
// Observe abort during the handshake wait, and never pool a poisoned instance: if the wait ends
|
||||
// in failure — an abort on a still-pending handshake, OR `initialize` rejecting (utf-8
|
||||
// negotiation, malformed result) without the process exiting — tear the instance down so a
|
||||
// permanently-rejecting/pending `ready` can't make every later query for this workspace fail.
|
||||
try {
|
||||
await this.abortable(this.ready, signal)
|
||||
} catch (error) {
|
||||
if (signal?.aborted && !this.dead) {
|
||||
if (!this.dead) {
|
||||
this.disposed = true
|
||||
await this.tearDown(abortError(signal))
|
||||
/* v8 ignore next -- ready rejects with an Error (abort reason or initialize failure); the String() fallback is defensive. */
|
||||
await this.tearDown(error instanceof Error ? error : new Error(String(error)))
|
||||
}
|
||||
throw error
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user