Extract connector from butter_stack with full history - #1
Merged
Conversation
…roup 1) The standalone proof for issue #1575 checkbox groups 0 and 1: a Go daemon a studio runs inside its own network, and the day-1 protocol schema that had to land before any verb did. New top-level `connector/`. Nothing else in the repo is touched: no /connect endpoint on the Rails app, no ActionCable change, no migration, no terraform, nothing deployed. Those are later PRs. What is here: - A daemon that dials out over WSS to one hostname on 443 and never listens on anything. The connector token travels in the Authorization header and nowhere else; an endpoint carrying a query string is refused at config load, so a copy-pasted ?token= URL cannot start the daemon (Shuri F1/F6, issue #935's lesson applied to a new surface before it exists). - A typed allowlist with a per-verb argument-constraint layer, in one readable file, because an IT director is asked to read the source. Two layers before any tool call: vocabulary membership, then a fixed per-verb argument schema where an unnamed key stops the command rather than being ignored into the call. Compiled: teamcity.server.info, teamcity.build.get, p4.describe, p4.changes, and the sys verbs. No mutating and no content-class verb is compiled in. - No caller-supplied trigger parameters, enforced structurally rather than by convention (Shuri F4). `bannedArgNames` lists the argument names no verb may declare, compiled or reserved, each with the reason; `Selfcheck()` runs in the tests and again at process start, so a build whose vocabulary grew one refuses to run. - Scoped arguments (depot_scope, allowed_build_types, repo_allowlist) that live only in connector.yml. A depot path's literal prefix must already sit inside a scoped prefix, so a wildcard cannot climb above the scope and `//...` is denied even to a P4 user who could read it (Shuri F5). - Credentials read only from connector.yml or a *_file path it names. No environment fallback, no flag that takes a secret, no remote configuration. 0600 or stricter, or it refuses to start. - p4 invoked as an argv array with no shell; the ticket passed via P4PASSWD in a minimal environment so it never appears in the host's process list. - A minimal RFC 6455 client written instead of a dependency, for the CVE surface and because it has to be readable. Only third-party dependency: yaml.v3. - A Ruby mock broker and the seven drills from design note 4.3, plus a round-trip phase and the broker-side half of drill (f). All nine pass. The Ruby WebSocket server half was written independently from the RFC, so a framing mistake fails a drill instead of agreeing with itself. PROTOCOL.md is checkbox group 0 written down, including the two rules this PR cannot enforce because they are Rails-side: the dedicated Rack endpoint (never ActionCable, never /cable, no change to allowed_request_origins) and the tenant scoping for Connector.call. README.md carries the "what this does not prove" list, which is the honest half: nothing ran against staging, demo, or prod; the argument-constraint layer is proved at the frame boundary against a mock, not end to end; conditions 1, 4 and 5 (signing, SBOM, version skew, the enforced egress spec) are untouched. Refs ButterStack/butter_stack#1575 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SHDQkRkNToRZ7BSM3YBQpN
…iveness Review follow-ups to the PR #1578 spike (issue #1575). - depot_scope was a bare prefix match, so a scope of //depot/game admitted //depot/gamesecret/... . config.Validate now normalizes every entry to a trailing slash and vocab.withinPrefixList matches only on a path-segment boundary (the scope root itself, or "prefix/"). Tests for both. - A failed tool call sent err.Error() to the broker inside the result frame; p4 stderr and Go url.Error carry the studio's LAN host, port and URL. The frame now carries the stable reason tool_error and the detail goes only to the local audit log. PROTOCOL.md sections 3 and 7 updated. - A healthy idle connector reconnected every ~10s: the read loop treated two silent heartbeat intervals as a dead socket, but the protocol has no broker-to-connector traffic while idle. The heartbeat tick now also sends a WebSocket ping, Conn tracks the last inbound frame of any kind (pong included), and a read deadline only fails the session when nothing at all arrived in the window. Surfaced by the UAT suite; drills never ran long enough to see it. - Em dashes replaced in lines this PR added. Upstream: PR #1578. Refs ButterStack/butter_stack#1575 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> (cherry picked from commit 3728e673cf88d18d1337148cda4a4cda3bf84ca8)
New `npm run test:uat:connector` (tests/uat/connector.spec.js) covering
TeamCity Tier 1 webhook intake and the ButterStack Connector spike, the
automated form of Devin's home-lab plan (design note section 4).
Topology (docker-compose.uat-connector.yml, profile `connector`): a
`studio_lan` network with internal: true holds a TeamCity stub and the
connector; the only ways out are the connector's own outbound wss:// leg
to a mock broker and the stub's webhook POSTs through a socat egress
forwarder, the model of a studio firewall that allows outbound only.
Phase 100 proves the shape: Rails cannot resolve or reach the stub, the
connector publishes no port and holds no listening socket, and the
connector can reach the stub.
Phases: TeamCity jenkins-type integration with ci_provider teamcity;
curl-step webhook (X-Webhook-Token) lands a BuildRun with the p4-<CL>
commit hash, the teamcity label and the stamped integration; the native
{eventType, payload} envelope with php-auth-pw authenticates and is
recorded but creates no BuildRun (dropped until #1574 Phase 1; the
assertion flips then); wrong or missing credential 401s; the connector
session comes up with the compiled verb list; sys/teamcity/p4 verbs round
trip (fake_p4 argv proves no shell); every denial reason (unknown verb,
out-of-scope path, reserved verb, string-for-int, content toggle, unknown
argument); query-string and missing-token upgrades are refused; a
cross-session result is discarded; stop/start, revoke and re-register
degrade and recover.
Also: connector/Dockerfile (multi-stage, non-root, no EXPOSE) with a
UAT-only entrypoint that renders connector.yml from env under umask 077
(the daemon itself still reads only the file); mock_broker_server.rb with
a small admin API; teamcity_stub.rb runnable standalone with an outbound
fire-webhook endpoint; a connector-go CI job (vet, test, build); README
UAT section listing what the suite does not prove (no Rails /connect
broker yet, no real TeamCity or p4d).
Upstream: PR #1578 (or its own test PR).
Refs ButterStack/butter_stack#1575
Refs ButterStack/butter_stack#1574
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
(cherry picked from commit 1d970c9e2c5846a56e6d43f4f0080f99e445da06)
Stage A against a real TeamCity 2026.1.3 (fixture report 2026-08-30-teamcity-fixture-stage-a.md) showed the built-in webhooks send a standard Authorization: Basic header built from teamcity.internal.webhooks.username/.password (plain parameter only; Password-typed sends no auth header), not the php-auth-* pair the plans assumed. The uat:connector stub's native mode now sends Basic with the real server's User-Agent by default and builds the envelope in the full REST Build shape from the captured payloads; auth: "php-auth" keeps the secondary channel covered. Spec phases: 410 native+Basic, new 415 native+php-auth, 420 covers wrong Basic, wrong php-auth and missing credential. Full uat:connector run and all nine drills green. Upstream: PR #1578 (rides the uat:connector commit). Refs ButterStack/butter_stack#1574 Refs ButterStack/butter_stack#1575 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> (cherry picked from commit 397db4b5f19e59121f26bea459c48bab1b18ba60)
Round-1 review, Ryan: GET / on the mock broker's admin port answered with a bare "no such admin route" error. It now returns a JSON index naming what the process is (the drill stand-in for the future Rails /connect broker, not the real broker), the wss:// endpoint, and each admin route with a one-line description. No route behavior changed. Upstream: PR #1578. Refs ButterStack/butter_stack#1575 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> (cherry picked from commit 312dac0a2843e331015da8858ee869caacbfc7b7)
…group 2b) Adds the Connector record (one active per project, v0), the token format bsc_<project.public_id>_<secret>, and a Connectors section on the project Integrations page (create/reveal-once/revoke). Scaffolding only: the /connect broker, verb execution, and tenant-scoped frame routing are a separate security-gated follow-up. Refs #1575
…tropy wording
Review + security fast-follow for PR #1589 (#1575 group 2b):
- Rescue ActiveRecord::RecordNotUnique in Projects::ConnectorsController#create
so a concurrent double-create against idx_one_active_connector_per_project
shows the same refusal alert as the app-level validation instead of a 500.
Extracted the shared message to Connector::ACTIVE_CONNECTOR_LIMIT_MESSAGE so
both refusal paths can never drift apart.
- Escape the connector name in the generated connector.yml snippet
(connector_id: #{name.to_json}) so names containing YAML-significant
characters (":", "#", quotes) can't produce a broken/reinterpreted file.
- Fix contrast on the one-connector note: --bs-fg-faint -> --bs-fg-mute.
- Correct connector/PROTOCOL.md's token entropy claim: the secret segment is
32 characters from a 58-symbol alphabet ([A-Za-z2-7]), not standard base32,
which is ~187 bits of entropy, not 256.
Refs #1575
…at/extract-from-butter-stack # Conflicts: # README.md
Bring the connector/ subtree from ButterStack/butter_stack into this repo as a standalone Go module, preserving all commit history via git subtree split. Add: - LICENSE (MIT, matching butterstack-cli) - .github/workflows/ci.yml (go vet, go test, go build) - docs/design-notes.md (spike scope and vocabulary table from README) - docs/uat.md (UAT topology and assertions from README) Rewrite README for a standalone repo reader: security model, install, configure (field-by-field from config.go), run (foreground, systemd, docker compose), verify (drills and make check), supported backends (compiled vs planned from vocab.go). Append mock-broker protocol rules to PROTOCOL.md. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
ryanlitalien
marked this pull request as ready for review
September 4, 2026 01:14
…repo goes public PROTOCOL.md and docs/ referred to internal review notes by the reviewer's agent name and by paths inside the private ButterStack repository. Those now read as "the design note" / "the security review" with the date, so the public text stands on its own. No technical content changed. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <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
1 participant
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.
Summary
Extract the ButterStack connector from
ButterStack/butter_stack(directoryconnector/on main) into this standalone repo, preserving full git history viagit subtree split.docs/Go build and test results
All passing on this branch:
README sections: where each moved
The old README mixed end-user documentation with design rationale, spike scope, and UAT topology. The rewrite separates concerns:
Shuri review status (2026-08-30-pr1589-connector-scaffolding-review.md)
Shuri's review covers PR #1589 (connector model + UI scaffolding in the Rails app), not the Go connector code in this repo. Her findings:
Section 1: LOW, CONFIRMED - RecordNotUnique race
app/models/connector.rb:98-104andapp/controllers/projects/connectors_controller.rb:28(Rails app, not in this repo)dbeae00e fix(connectors): RecordNotUnique race, YAML-unsafe name, contrast, entropy wording) directly addresses this by name. The fix itself is in the Rails app code, not in the Go connector extracted here, so it is outside this PR's scope.Latent risks for Task D (not bugs, flagged for future)
Connector.authenticate's full-table scan (connector.rb:49-50): Rails-side, not in this repo. Flagged for Task D's broker implementation.32 characters drawn from a 58-symbol alphabet, with the entropy calculation), so this appears resolved./connectendpoint: server-side, not in this repo. Task D owes this.Test plan
go vet ./...passesgo test ./...passes (config, vocab, wsclient)go build ./cmd/butterstack-connectorsucceedsGenerated with Claude Code