Repository navigation
harden the ipc transport used by background jobs - #330
Open
kevinushey wants to merge 1 commit into
Open
kevinushey wants to merge 1 commit into
kevinushey wants to merge 1 commit into
Conversation
Adds a version 2 request format used when RStudio advertises it via RSTUDIOAPI_IPC_VERSION: a plain-text ticket (secret + request id) with the serialized call in a sibling payload file, so RStudio can verify the secret before deserializing anything, and a response of list(id, value | error) so stale responses are discarded. Falls back to the existing format for older RStudio versions. The shared secret is moved from the environment into an option when the package loads, so processes spawned by a job no longer inherit the ability to call into the IDE.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hardens the file-based IPC that R processes launched by RStudio (background jobs, renders, and so on) use to call back into the IDE. Companion RStudio change to follow; this PR is independently releasable and is compatible with every RStudio version in either order.
What changes
Version 2 request format. When RStudio advertises support via
RSTUDIOAPI_IPC_VERSION,callRemote()writes a plain-text ticket (magic line,version=2,secret=,id=) to the requests path and the serialized call to a sibling<requests>.payloadfile. The payload is written first and the ticket last, each via temp file + rename, so a complete ticket implies a complete payload. This lets RStudio verify the secret before deserializing anything. The response islist(id, value)orlist(id, error); the client discards responses whose id doesn't match, so a stale response from a timed-out call can't be mistaken for the current one.Fallback. Without
RSTUDIOAPI_IPC_VERSION, the existing single-RDS format and bare response are used, so new rstudioapi works with older RStudio. Both paths now tolerate a partially written or stale response file by discarding it and continuing to poll until the timeout.Secret scrubbed from the environment.
.onLoad()movesRSTUDIOAPI_IPC_SHARED_SECRETinto therstudioapi.ipc.secretoption and unsets it. Previously every process a job spawned (callr workers, compilers, package managers, shell tools) inherited a token that could evaluate arbitrary R in the IDE session. The request and response path variables are left in place, soisJob()and third-party job detection are unaffected. A spawned R process that tries to call rstudioapi now gets a clear error instead of silently reaching the IDE.Tests
tests/testthat/test-remote.RrunscallRemote()against a fake RStudio implemented in a backgroundRscript, covering the v2 ticket/payload layout and id matching, the v1 fallback, error propagation, timeout, missing credentials, and the.onLoad()scrub.Verified end to end against an RStudio build with the server side of this change: a background job calling
rstudioapi::versionInfo()succeeds via both the legacy and the v2 path.