From 6b910375706e214438ceeba85ebf8ea73ba0f8bf Mon Sep 17 00:00:00 2001 From: Ryan L'Italien Date: Wed, 9 Sep 2026 16:52:06 -0400 Subject: [PATCH 1/2] release: set HOME in the image, and drop the pre-release framing Two things, both surfaced by the first production deployment. HOME is load-bearing and the image never set it. The daemon passes HOME straight through to `p4`, which reads $HOME/.p4trust and $HOME/.p4tickets. With HOME empty, p4 never finds the trust file and every p4.* verb fails against an SSL-enabled p4d with `exit status 1`, even when the mount is correct. The first studio deployment hit this and worked around it with a compose-level `HOME:` env var; that belongs in the image, so nobody else has to rediscover it. Set on both the runtime and uat stages, so moving between them still changes only the digest. The docs still described a day-1 spike against a mock broker. The Perforce path now runs in production: a real changelist travelled from a studio's own Helix Core, through the connector, over one outbound TLS connection to the broker, and was recorded on a ButterStack project. So: - README drops "Status: pre-release" and the link to a tracking issue in a private repo (a 404 for every public reader), and says what is actually proven. - PROTOCOL.md is v0, the protocol the shipped connector speaks, not a schema waiting for a server half. The server half exists. - design-notes.md's "What the spike does not prove" becomes "What is not yet proven", with the items production closed out moved into a "proven since" note and the honest remainder kept: the drills have no production-side result, TeamCity is off until a connector-scoped credential exists, home-connection latency, Sigstore signing and egress.md, scale and multi-node routing, and the uncompiled verbs. --- Dockerfile | 10 ++++++++++ PROTOCOL.md | 21 +++++++++++---------- README.md | 2 +- docs/design-notes.md | 24 +++++++++++++----------- 4 files changed, 35 insertions(+), 22 deletions(-) diff --git a/Dockerfile b/Dockerfile index 50b47b0..24b46c4 100644 --- a/Dockerfile +++ b/Dockerfile @@ -56,6 +56,11 @@ RUN chmod 0755 /usr/local/bin/butterstack-connector /usr/local/bin/uat-entrypoin USER connector:connector WORKDIR /home/connector +# HOME is load-bearing: the daemon passes it straight through to `p4`, which +# reads $HOME/.p4trust and $HOME/.p4tickets. Without it p4 gets an empty HOME, +# never finds the trust file, and every p4.* verb fails against an SSL-enabled +# p4d with `exit status 1`. +ENV HOME=/home/connector # Production entrypoint: reads connector.yml from the path a studio wrote. # The UAT compose service overrides this with uat-entrypoint.sh, which @@ -116,5 +121,10 @@ COPY --from=build /out/butterstack-connector /usr/local/bin/butterstack-connecto # could overwrite. USER 10001:10001 WORKDIR /home/connector +# HOME is load-bearing: the daemon passes it straight through to `p4`, which +# reads $HOME/.p4trust and $HOME/.p4tickets. Without it p4 gets an empty HOME, +# never finds the trust file, and every p4.* verb fails against an SSL-enabled +# p4d with `exit status 1`. +ENV HOME=/home/connector ENTRYPOINT ["/usr/local/bin/butterstack-connector", "-config", "/etc/butterstack/connector.yml"] diff --git a/PROTOCOL.md b/PROTOCOL.md index 625691a..1c94a85 100644 --- a/PROTOCOL.md +++ b/PROTOCOL.md @@ -1,19 +1,19 @@ # ButterStack Connector protocol, v0 -Status: **day-1 spike schema** for issue #1575 group 1. This document is the -written-down form of checkbox group 0 (the day-1 protocol schema), which had to -land before any verb did, because argument constraints *are* schema. +Status: **v0**, the protocol the shipped connector speaks. Argument constraints +*are* schema, so this document is the normative reference for both halves of the +wire: change a constraint here before you change it in code. Sources: the ButterStack team's design note §2.2 and §2.4 (the ButterStack connector design note (2026-08-29, internal), an internal planning branch) and the security review's must-fix list §6 (the connector security review (2026-08-29, internal)). -Two things this document does **not** do. It does not describe an endpoint that -exists: the broker side is a later PR, and the only implementation of this -protocol's server half today is the drill harness in `test/mock_broker.rb`. And -it does not restate the six survival conditions; it records the parts of them -that are schema. +One thing this document does **not** do: it does not restate the six survival +conditions, only the parts of them that are schema. The server half of this +protocol is the ButterStack broker at `wss://connect.butterstack.com/connect`; +`test/mock_broker.rb` is a second, local implementation of the same half, used +by the drill harness. --- @@ -219,8 +219,9 @@ missed scope returns cross-tenant rows *silently* instead of raising. Therefore: - a `result` frame is dropped unless it matches the issuing session. The drill: assert tenant context is nil at the start of a request that follows a -connector frame on the same Puma thread. **That drill is Rails-side and is not -covered by this PR** (see `README.md`, "what this does not prove"). +connector frame on the same Puma thread. That drill is broker-side, so it lives +with the broker rather than in this repo (see +[docs/design-notes.md](docs/design-notes.md), "What is not yet proven"). --- diff --git a/README.md b/README.md index 16acc03..13310ef 100644 --- a/README.md +++ b/README.md @@ -2,7 +2,7 @@ An outbound-only daemon a game studio runs inside its own network so [ButterStack](https://butterstack.com/?utm_source=github&utm_medium=readme&utm_campaign=butterstack-connector) can reach a private, on-premises Perforce, TeamCity, Jenkins, GitHub Enterprise Server, or Horde without the studio opening a single inbound port. One outbound TLS connection to one hostname on 443. A typed command allowlist, never a tunnel and never a shell. Credentials stay on the studio's disk and never cross the wire. -Status: pre-release. Tracking: [ButterStack/butter_stack#1575](https://github.com/ButterStack/butter_stack/issues/1575). +Status: released, and running in production. The Perforce path is proven end to end against a live Helix Core server: a real changelist reached a ButterStack project through the broker, with no inbound port opened anywhere on the studio's network. [Supported backends](#supported-backends) records what each backend covers today. ## What it is diff --git a/docs/design-notes.md b/docs/design-notes.md index f1cc024..9fad110 100644 --- a/docs/design-notes.md +++ b/docs/design-notes.md @@ -1,6 +1,6 @@ # Design notes -This file preserves the design rationale and review history that was originally in the README when the connector lived inside the butter_stack monorepo as the issue #1575 spike. +This file preserves the design rationale and review history that was originally in the README, from when the connector lived inside the ButterStack monorepo and before it was extracted into this repository. ## Design sources @@ -22,18 +22,20 @@ the connector security review (2026-08-29, internal) section 6. No verb accepts a host, port, URL, or shell string. No verb accepts caller-supplied build parameters or properties, that is enforced structurally (`bannedArgNames` plus `Selfcheck()`, which runs at process start as well as in the tests), because a parameter map on a build-triggering verb interpolates into shell build steps and would make the allowlist a code-execution primitive inside the studio's LAN. No mutating verb and no content-class verb is compiled in. -## What the spike does not prove +## What is not yet proven -Carried forward from design note section 5 and security review section 6 item 7, plus what the standalone shape adds: +This list started as the spike's go/no-go input and is kept current as things are proven, so it is a running record of what is claimed and what is not. -- **The argument-constraint layer end to end.** The drills prove denial at the frame boundary against a mock broker. They do not prove it against a real broker, a real TeamCity, or a real p4d. -- **Anything on the Rails side.** There is no `/connect` endpoint, no ActionCable change, no migration, no UI. The tenant-context drill ("assert tenant context is nil at the start of a request that follows a connector frame on the same Puma thread") is Rails-side and is not covered here. Only the broker-side half of drill (f) is. -- **Anything on real infrastructure.** Nothing ran against staging, demo, or production. No terraform, no security group, no hostname, no certificate. Stage A (the Tier 1 TeamCity webhook run) has not been run, and it is gated on the #1574 Phase -1 app fixes landing first; the "no token in `webhook_events.payload` or the app log" drill therefore has no result yet. -- **The frame codec against an independent production stack.** Both ends here were written from RFC 6455, the Go client and the Ruby server independently, which is why a masking or handshake mistake shows up as a failed drill. But neither has met a real ALB, a real nginx `Upgrade` hop, or a real proxy. -- **Latency over a home connection.** The drills run on loopback. The design's under-2-second target is untested against a NATed home network, and the `ss`/`netstat` capture showing exactly one outbound established connection and zero listeners has not been taken. -- **Survival conditions 1, 4, and 5.** No Sigstore keyless signing, no SBOM, no build-from-source instructions, no digest-pinned base image, no version-skew handling, and no `egress.md` with a per-verb output schema enforced as a field allowlist with a conformance test. The fixed `fields=` projections in the TeamCity executor are the beginning of that, not the whole of it. -- **Scale and multi-node routing.** Puma behaviour at tens of connectors, socket routing under a real ASG scale-out, and the per-integration connection cap and per-session command budget in the broker. +**Proven since:** on 2026-09-08 a real Perforce changelist travelled from a studio's own Helix Core server, through a connector on the studio's box, over one outbound TLS connection to the production broker, and was recorded on a ButterStack project - which closed out, in one run, the argument-constraint layer against a real p4d, the frame codec against a real ALB, and the broker half existing at all. The published image is digest-pinnable and each release tag carries build provenance and an SBOM. + +Still open: + +- **The seven drills against the production broker.** They pass against `test/mock_broker.rb` on loopback. The production run has been exercised by real traffic rather than by the drill harness, so the denial paths in particular have no production-side result. +- **TeamCity in production.** The verbs are compiled and drilled, but the one live deployment runs with `teamcity.enabled: false`, because handing the daemon an admin-scoped TeamCity token would give any `teamcity.*` verb an admin session's blast radius. This turns on once a connector-scoped TeamCity credential exists. +- **Latency over a home connection.** The drills run on loopback. The design's under-2-second target is untested against a NATed home network. +- **Survival conditions 1 and 5.** No Sigstore keyless signing, and no `egress.md` with a per-verb output schema enforced as a field allowlist with a conformance test. The fixed `fields=` projections in the TeamCity executor are the beginning of that, not the whole of it. +- **Scale and multi-node routing.** Broker behaviour at tens of connectors, socket routing under a real scale-out, and the per-integration connection cap and per-session command budget. - **Everything beyond the five compiled verbs.** No Jenkins, GHES, or Horde verb; no Perforce verb beyond `describe` and `changes`; no mutating verb; no content verb; no poll-loop mode; no Windows service. - **An actual IT-director review.** Appendix B is a script, not a test. -This is the go/no-go input for the build, and it is deliberately smaller than the product. +The scope is deliberately smaller than the product, and that is the point: every verb that is not compiled in cannot be executed, whatever the broker asks for. From e76c57e023a6c353ae94fcfabf26f61a8487f135 Mon Sep 17 00:00:00 2001 From: Ryan L'Italien Date: Wed, 9 Sep 2026 17:02:49 -0400 Subject: [PATCH 2/2] repo: SECURITY.md, CONTRIBUTING.md, and issue templates The repo had a LICENSE and nothing else. Every sibling public repo (butterstack-cli, butterstack-mcp, gamedev-agents, perforce-docker) carries CONTRIBUTING.md and issue templates, and the two that handle credentials carry SECURITY.md. This one handles credentials more directly than any of them and had no disclosure path at all, so a reporter's only option was a public issue. SECURITY.md is written around the daemon's four standing claims (outbound only, the allowlist is the boundary, credentials stay local, the broker cannot reconfigure the connector), so a reporter can tell whether what they found is in scope. It also says what is not a security issue, since a reserved verb returning a denial and a hard startup failure on a bad config are both working as designed and both look alarming from the outside. CONTRIBUTING.md leads with the constraint that actually governs changes here: the vocabulary is the security boundary, argument constraints are schema, and a denial path with no drill is an untested security claim. The bug template asks for the connector log and a redacted connector.yml, and warns before the paste rather than after. Two accuracy fixes found while writing these: - README asked for Go 1.25 to build from source; the module's own `go` directive is 1.23. Release builds do use 1.25, so both numbers are now stated for what each one is. - `make check` already runs both test and drills, so telling contributors to run them separately was wrong. --- .github/ISSUE_TEMPLATE/bug_report.md | 49 +++++++++++++++++++++ .github/ISSUE_TEMPLATE/config.yml | 8 ++++ .github/ISSUE_TEMPLATE/feature_request.md | 23 ++++++++++ CONTRIBUTING.md | 52 +++++++++++++++++++++++ README.md | 2 +- SECURITY.md | 25 +++++++++++ 6 files changed, 158 insertions(+), 1 deletion(-) create mode 100644 .github/ISSUE_TEMPLATE/bug_report.md create mode 100644 .github/ISSUE_TEMPLATE/config.yml create mode 100644 .github/ISSUE_TEMPLATE/feature_request.md create mode 100644 CONTRIBUTING.md create mode 100644 SECURITY.md diff --git a/.github/ISSUE_TEMPLATE/bug_report.md b/.github/ISSUE_TEMPLATE/bug_report.md new file mode 100644 index 0000000..b3d05ad --- /dev/null +++ b/.github/ISSUE_TEMPLATE/bug_report.md @@ -0,0 +1,49 @@ +--- +name: Bug report +about: Something isn't working the way it should +title: "" +labels: bug +assignees: "" +--- + +**What happened** + +A clear description of the bug. + +**What you expected** + +What you expected to happen instead. + +**Steps to reproduce** + +1. +2. +3. + +**Connector log** + +``` +paste the relevant lines from the daemon's log here + +The daemon redacts token and ticket values as [redacted], but please read +before pasting anyway: do not include a token, a ticket, or anything else +you would not put in a public issue. +``` + +**Your connector.yml, with secrets removed** + +```yaml +# endpoint, connector_id, scopes, and the perforce/teamcity sections are the +# useful part. Remove token, token_file contents, and any ticket value. +``` + +**Environment** + +- Connector version: (the image tag you ran, or `butterstack-connector -version`) +- How you run it: (published image, locally built image, or binary) +- Backend and version: (Perforce/Helix Core, TeamCity, etc.) +- Host OS: + +**Additional context** + +Anything else worth knowing. diff --git a/.github/ISSUE_TEMPLATE/config.yml b/.github/ISSUE_TEMPLATE/config.yml new file mode 100644 index 0000000..e2e7acf --- /dev/null +++ b/.github/ISSUE_TEMPLATE/config.yml @@ -0,0 +1,8 @@ +blank_issues_enabled: true +contact_links: + - name: Security issue + url: https://github.com/ButterStack/butterstack-connector/blob/main/SECURITY.md + about: Please do not open a public issue for a security problem. Email hello@butterstack.com instead. + - name: ButterStack support + url: https://www.butterstack.com/ + about: Questions about a ButterStack account, a project, or the Connectors tab. diff --git a/.github/ISSUE_TEMPLATE/feature_request.md b/.github/ISSUE_TEMPLATE/feature_request.md new file mode 100644 index 0000000..8103c72 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/feature_request.md @@ -0,0 +1,23 @@ +--- +name: Feature request +about: Suggest an idea or improvement +title: "" +labels: enhancement +assignees: "" +--- + +**What problem does this solve** + +A clear description of the problem or gap. What are you trying to let ButterStack see that the connector does not reach today? + +**Proposed solution** + +What you'd like to see. If you are proposing a new verb, please read `PROTOCOL.md` section 4 first and include the verb name and its arguments, with the constraint on each one. + +**Alternatives considered** + +Any other approaches you thought about, and why they don't fit as well. + +**Additional context** + +Anything else worth knowing. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 0000000..033761f --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,52 @@ +# Contributing + +Thanks for taking a look at the ButterStack Connector. + +## Project shape + +A single static Go binary with no runtime dependencies beyond the tools it shells out to (`p4`). Ruby appears in exactly one place, the drill harness in `test/`, and it stays there: the shipped runtime image has no Ruby in it. + +The load-bearing constraint of this codebase is that **the allowlist is the security boundary**. `internal/vocab/vocab.go` is the authoritative list of what a broker can ask the daemon to do, and `PROTOCOL.md` is its written form. Argument constraints are schema, so a change to one is a change to both. + +Before you propose a new verb, read `PROTOCOL.md` section 4 and `docs/design-notes.md`. A verb that accepts a host, port, URL, shell string, or caller-supplied build parameters will not be accepted: `bannedArgNames` and `Selfcheck()` exist to make that structural rather than a matter of review attention, and `Selfcheck()` runs at process start as well as in the tests. + +## Getting set up + +``` +git clone https://github.com/ButterStack/butterstack-connector.git +cd butterstack-connector +make build +``` + +Requires Go 1.23 or later (the `go` directive in `go.mod`); release builds use 1.25. Ruby 3.2+ is needed only to run the drills. + +## Running the tests + +``` +make test # gofmt, go vet, go test +make drills # the seven drills against test/mock_broker.rb +make check # both of the above, and what CI runs +``` + +The drills run entirely on loopback in a temporary directory: a throwaway CA and server certificate so the connector dials a real `wss://` endpoint with real certificate verification, a stub TeamCity that refuses any request not carrying the token from `connector.yml`, and a fake `p4` that records its own argv. They cover the round trip plus out-of-vocabulary verbs, out-of-scope arguments, a query-string token, cross-session results, and four recovery conditions (network drop, connector stopped, broker stopped, token revoked). + +If you touch the frame codec, the vocabulary, or the argument-constraint layer, run `make drills` and expect to add one. Several drills exist specifically to prove a denial path, and a denial path with no drill is an untested security claim. + +## Reporting issues + +Please use the issue templates. For anything that looks like a security issue, do not open a public issue: see [SECURITY.md](./SECURITY.md). + +## Pull requests + +- Keep changes focused and explain the "why," not just the "what." +- Add or update drills for behavior changes, and update `PROTOCOL.md` in the same PR if you changed anything a broker can observe. +- Run `make check` before opening a PR. +- Match the surrounding code style; there is no external formatter dependency beyond `gofmt`. + +## Releases + +Releases are tag-driven. Pushing a `v*` tag builds the per-OS/arch archives, publishes the container image to `ghcr.io/butterstack/butterstack-connector`, and attaches provenance and an SBOM. Merging to `main` publishes nothing on its own. + +## Code of conduct + +Be respectful and constructive. This is a small project maintained alongside a larger product; response times may vary. diff --git a/README.md b/README.md index 13310ef..2f539f3 100644 --- a/README.md +++ b/README.md @@ -10,7 +10,7 @@ The connector is a daemon the studio runs on its own hardware (or in a container ## Requirements -- Go 1.25 or later (for building from source) +- Go 1.23 or later (for building from source; release builds use 1.25) - Ruby 3.2+ (only for running the drill harness in `test/`) - A [ButterStack account](https://butterstack.com/users/sign_up?utm_source=github&utm_medium=readme&utm_campaign=butterstack-connector) with a connector token issued from the project's Connectors UI diff --git a/SECURITY.md b/SECURITY.md new file mode 100644 index 0000000..c0d76d6 --- /dev/null +++ b/SECURITY.md @@ -0,0 +1,25 @@ +# Security Policy + +The connector is a piece of security infrastructure: it exists so a studio can give ButterStack read access to a private Perforce, TeamCity, Jenkins, GitHub Enterprise Server, or Horde server without opening an inbound port or handing over a credential. A defect here is a defect in that boundary, so please report one privately rather than in a public issue. + +Email hello@butterstack.com with details. We will acknowledge receipt and work with you on a fix and a disclosure timeline. + +## What we consider a security issue here + +Anything that breaks one of the daemon's four standing claims: + +1. **Outbound only.** The daemon opens exactly one outbound TLS connection and never listens. Anything that makes it accept an inbound connection, bind a port, or act as a tunnel. +2. **The allowlist is the boundary.** Only verbs compiled into `internal/vocab/vocab.go` can execute, with constrained arguments. Anything that executes a verb outside the compiled vocabulary, gets an argument past its constraint, reaches a shell, or turns a verb into a code-execution primitive inside the studio's network. A verb accepting a host, port, URL, or shell string would be one of these. +3. **Credentials stay local.** Perforce tickets and TeamCity tokens live in the studio's own files and are never transmitted. Anything that puts a credential, a LAN hostname, a port, or an internal URL into a frame, an error message, or the audit log. +4. **The broker cannot reconfigure the connector.** Configuration comes from `connector.yml` on the studio's disk. Anything that lets a broker frame change what the daemon is allowed to do or where it looks for a credential. + +Findings against the ButterStack broker side (`wss://connect.butterstack.com/connect`) are also in scope and go to the same address. + +## What is not a security issue + +- A reserved verb returning a denial. `jenkins.build.trigger`, `ghes.commit.get`, `horde.server.info`, `teamcity.build.queue`, and `p4.file_contents` are in the vocabulary so the schema is self-documenting and the drills exercise their denial path. They are not executable in this build, by design. +- The daemon refusing a `ws://` endpoint, a malformed `connector.yml`, or an unknown YAML key. Those are intentional hard failures at startup. + +## Supported versions + +This project is pre-1.0. Security fixes land on the latest published tag; there is no separate maintenance branch at this stage. Pin an exact version tag rather than `latest`, and watch the Releases page.