Repository navigation
docs: say what an embedded Run() can reach, and stop claiming nothing is shared between calls - #770
Merged
Merged
Conversation
… is shared
The embedding section promises its contract "as measured rather than intended".
Two of its statements were not what the code does, and a third capability was
not mentioned at all. Each correction below is a measurement against
origin/main through the public `Run` and nothing else.
"Nothing is shared between calls" is true of the Ruby heap and false of the
process. A script that assigns to `ENV` and calls `Dir.chdir` changes them for
the host Go program and they stay changed after `Run` returns: a host whose
PATH was 860 bytes and whose cwd was its build directory came back with a
13-byte PATH and a cwd of /private/tmp. The bullet immediately below it already
documented that `require_relative` resolves against the *process* working
directory, so the README contained the fact that contradicted it; one `Run` can
therefore plant a file a later `Run` loads. The bullet is now scoped to the
heap, which is the part that holds -- a global, a constant and a String
monkey-patch really are gone in the next call.
`exit`, `exit!` and `abort` make `Run` return nil, so an embedder cannot tell
"finished" from "stopped": `abort "fatal"` writes its message to `out` and
reports success, and `exit 3` loses the 3. Only a Ruby exception becomes a Go
error. Said explicitly, with the reason it cannot be worked around today --
the CLI recovers the status through `internal/vm`, which is unreachable.
`Kernel#fork` exists and runs the block in the same process, returning a
synthetic pid while `Process.pid` is unchanged, so the "child" mutates the
parent's globals, ENV and cwd. The README said only that `Process.fork` does
not exist, which reads as "fork is unavailable" -- but `fork { }` is the
spelling almost every program uses, and code that forks to isolate or to drop
privileges gets neither. Added as a divergence row and cross-referenced from
the Process section.
Finally a "Run is not a sandbox" subsection. This is not a defect report: MRI
is not sandboxed either and conformance is the goal. It is worth stating
because MRI is normally a CLI a person runs on their own code whereas `Run` is
a function someone links into a server. It also records the two things that are
deliberately out of reach -- no path from Ruby to the host's Go state, and the
host's standard descriptors are not exposed, with IO.for_fd(1) raising EBADF
where MRI writes to the real descriptor -- because those are properties worth
not losing by accident.
The new test file pins all of it, in the spirit of #765: the prose cannot drift
from the code silently, and each failure is the signal to rewrite the bullet.
The raise-becomes-an-error twin is there on purpose -- without it the
exit-returns-nil test would also pass if `Run` swallowed every error.
Refs #768.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This was referenced Oct 4, 2026
tannevaled
added a commit
to go-embedded-ruby/docs
that referenced
this pull request
Oct 4, 2026
…cess state is (#19) The "Embedding it in a Go program" section promised, unqualified, that "nothing is shared between `Run`s" -- and then said, in the same sentence, that `require_relative` resolves against the **process** working directory. Both cannot be true, and the measurement says the second one is. Measured against origin/main through the public `Run` and nothing else, with a small Go embedder rather than the rbgo CLI: a script that assigns to `ENV` and calls `Dir.chdir` changes them for the host Go program and they stay changed after `Run` returns. A host whose PATH was 860 bytes and whose cwd was its build directory came back with a 13-byte PATH and a cwd of /private/tmp. Since `require_relative` anchors there, one `Run` can also plant a file a later `Run` loads. The narrow claim does hold and is kept: a global, a constant and a String monkey-patch set in one call really are gone in the next. So the sentence is now scoped to the Ruby heap instead of being dropped. Two further corrections in the same paragraph: `exit`, `exit!` and `abort` make `Run` return nil, so the "a Ruby exception comes back as a Go error" clause was describing the one case that works. Tracked upstream in go-embedded-ruby/ruby#773. And a warning admonition saying plainly that `Run` is not a sandbox. That is not a defect report -- MRI is not sandboxed either and conformance is the goal -- but MRI is normally a CLI a person runs on their own code, whereas `Run` is a function someone links into a server, and this page had no safety language at all. It also records the two things that are deliberately out of reach, because those are properties worth not losing by accident. The measurements live in the repository README rather than being duplicated here, and the admonition links to them. Its anchor resolves once go-embedded-ruby/ruby#770 merges, which adds that section. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
tannevaled
added a commit
that referenced
this pull request
Oct 9, 2026
… now public (#780) There is no SECURITY.md in this repository and none in the organisation's .github repository, so a researcher who finds something has nowhere to send it except the public issue tracker. That mattered less before today; the security audit in #768 has just filed nine findings in the open, which is an invitation for the next person to do the same with something worse. The policy is deliberately modest about response: this is a small project with no security team, and promising a timeline nobody is on call for would be worth less than saying so. Two parts carry the actual content. What is IN scope is written from what the audit actually found, not from a template: the host process dying or hanging without bound on Ruby input (because `Run` returns an error, and an embedder cannot defend against a `fatal error:` from outside); failing to raise where MRI raises when a program relies on the raise to stay safe -- the fail-open class, which is invisible to the caller; reaching past the documented capability set; and the CI/release path. What is NOT a vulnerability is the part that keeps the inbox usable. `Run` is not a sandbox, and that is documented rather than accidental, so "system() executes a command" is working as intended. By the same reasoning, faithfully reproducing a behaviour that is dangerous in MRI -- Marshal.load on hostile bytes -- is conformance here, though it is still worth hearing if the documentation fails to warn about it. It also lists the six already-filed classes, so a duplicate costs the reporter nothing to avoid. The channel is GitHub's private vulnerability reporting, which is currently DISABLED on this repository and has to be turned on for the policy to be truthful -- see the pull request. It was chosen over an email address because it does not publish anyone's personal address on a public repository. The link to the README's "Run is not a sandbox" section resolves once #770 merges, which is what adds that heading. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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.
Corrects the embedding section of the README against what the code does, and adds the capability statement it was missing. Comes out of a security audit of
rbgo; the behavioural half is tracked separately in #768.Every number and quoted output below I measured myself, against
origin/main(610a64a), through the publicRunand nothing else — a four-line Go embedder, not therbgoCLI, becausecmd/rbgoconfigures thingsRundoes not.1. "Nothing is shared between calls" was true of the heap and false of the process
The bullet said, unqualified:
Measured. A script that does nothing but touch process state:
The host Go process's working directory moved, its
PATHwent from 860 bytes to 13, andVMARKwas set — and it all stayed that way afterRunreturnednil.What makes this a documentation defect rather than a surprise is that the next bullet already said so:
So the README held the fact that contradicted its own claim, two lines apart. The two together are also a cross-call code path: one
Runcan write a file that a laterRunloads.The narrow reading does hold, and I checked it rather than assuming the whole bullet was wrong — a global, a constant and a
Stringmonkey-patch from call #1 are all absent in call #2. So the bullet is now scoped to the heap instead of being removed.2.
exit/exit!/abortreport successMRI exits 1 / 3 / 42 / 1. An embedder cannot distinguish "finished" from "stopped", and
abort's message lands inoutlooking like ordinary output. The README documented only theraisecase, which is the one that works. Now stated, with why it is not something an embedder can work around:cmd/rbgo/exit_status.gorecovers the status throughmachine.TerminalExit(), butrun.godiscards the*vm.VMandinternal/vmis unreachable from outside.3.
Kernel#forkexists, and does not forkThe README said only:
True, and it reads as "fork is unavailable". But
fork { }— the spelling almost every program uses — exists:The "child" overwrote the parent's global and the pid is synthetic.
internal/vm/spawn.gosays so outright — "Without an OS fork we run the block in-process". Code that forks to isolate or to drop privileges gets neither, which is why this belongs in the divergence table rather than in a footnote.4. A "
Runis not a sandbox" subsectionNot a defect report: MRI is not sandboxed either, and conformance is the goal. It is worth saying because MRI is normally a CLI a person invokes on their own code, whereas
Runis a function someone links into a server — and the README had no safety language at all (I searched forsandbox|untrusted|isolat|taint|$SAFE|secure; every hit wassecurerandomor therbgo_closedbuild tag).Measured through
Run, with MRI side by side:File.read("/etc/passwd")Dir.glob("~/.ssh/*")File.symlinkthen readTCPServer+TCPSocketround trip"PONG""PONG"require "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/abs/path.rb"system/ backticks /%x{}/IO.popen/Process.spawnIO.for_fd(1).write(...)Errno::EBADFdefined?(RubyVM)nil"constant"require "fiddle"LoadErrorObjectSpace.each_objectObjectSpace.each_object(String)Shell metacharacters go to
/bin/sh -c(internal/vm/spawn_native.go,shellish), so pipes,;,$(...), globs and redirection all work.The last two columns are the reason the section also records what is deliberately out of reach: no
RubyVM,ObjectSpaceis effectively a stub,fiddlewill not load, CGO is 0 — so there is no path from Ruby to the host's Go state — and the host's standard descriptors are not exposed. ThatIO.for_fd(1)raisesEBADFwhere MRI writes to the real descriptor is a genuine property worth not losing by accident: a script cannot write outside theoutthe embedder passed.I had drafted two further bullets (
UDPSocket/Net::HTTP, and reading the host's own executable) from a second measurement that hung before returning, so I narrowed both claims to what I had actually seen rather than publish them. If the socket probe turns out to hang inside rbgo that is a finding of its own, and it will get its own issue.Tests
readme_embedding_contract_test.gopins all four statements, in the spirit of #765 — the prose cannot drift from the code silently, and a failure is the signal to rewrite the bullet rather than the test.The
raise-becomes-an-error test is there on purpose: without that twin, the exit-returns-nil test would also pass ifRunswallowed every error. The cwd half of statement 1 is not tested — changing the test binary's working directory would disturb every other test in the package — and the test says so where it would be looked for.ENVis exercised throught.Setenv, so the framework restores it.gofmt -l .is empty.🤖 Generated with Claude Code