CONTRIBUTING.md
Rama main · réplica de lectura del repositorio original.
# Contributing to Kadran
Thanks for considering it. This document is short on ceremony and specific about
the one thing that matters most in this codebase: **the privileged surface**.
## The house rule
> If you write a comment claiming a security property, and an experiment could
> falsify that comment, write the experiment.
This is not a style preference. It keeps catching real bugs in this project,
including several where the comment and the code disagreed and only the comment was
read. A few examples now living in `docs/decisions.md`:
- A comment said transactions were immediate; the code opened deferred ones.
- A generated-code setting's comment said the opposite of what the setting did,
which would have let a new privileged RPC silently return `Unimplemented`.
- A forbidden-field check could not match `map<...>` or `optional` fields, so it
passed on schemas it was written to reject.
A test that has never been observed to fail is not evidence. **Break the fix,
watch the test go red, put it back.** If you can't make a test fail, say so in the
PR rather than shipping a green check that proves nothing.
## Changes to the privileged surface
Anything touching these is held to a higher bar:
- `proto/kadran/v1/exec.proto`
- `internal/exec/**`
- `cmd/kadran-exec/**`
- `deploy/systemd/**`
A PR here must include:
1. **A written threat rationale.** Answer the question the schema file asks:
*"If `kadrand` were fully compromised, what would it do with this field?"*
2. **A test for each new invariant,** including escape attempts, not just the
happy path.
3. **Evidence the test fires.** Describe the mutation you applied and the failure
you observed.
4. **Room in the budget.** `scripts/check-exec-surface.sh` caps the privileged code
at 2500 lines of code (comments and blanks excluded), measured from the real
import graph of `cmd/kadran-exec`.
Raising that cap is a decision, not a fix — open an issue first.
New forbidden field patterns must be added to `scripts/check-exec-surface.sh`. The
list is exported via `--list-forbidden` and the test script generates a proof for
every entry, so a pattern without a proof is structurally impossible. Keep it that way.
## Everyday workflow
```bash
go build ./...
go test -race ./...
gofmt -l . # must print nothing
golangci-lint run ./... # and GOOS=windows golangci-lint run ./...
buf lint && buf format --diff --exit-code # after editing proto/
buf generate # after editing proto/
scripts/check-exec-surface.sh
scripts/check-exec-surface-test.sh
for s in scripts/mutate-*.sh; do bash "$s"; done
```
A mutation script breaks one protection on purpose and requires a test to fail.
When you add an invariant, add a mutation for it. The mutant must compile — the
scripts refuse to count a build failure as "caught", and CI checks that every
script has that gate.
On a pull request CI runs only the mutation scripts your change touches:
`scripts/mutation-select.sh` picks them from the paths each script names, and picks
all of them when `.github/`, `proto/` or a `go.mod` changes. A change to `*.md` and
`docs/` alone runs none. All of them run every night, and on the commit to tag
before a release (`gh workflow run ci.yml --ref main`), because together they take
over an hour.
`buf generate` succeeding does **not** mean `buf lint` will. Style rules —
for example, a streaming RPC's response message must be named
`<Rpc>Response` — are only enforced by `buf lint`, and CI runs it.
Generated protobuf code is gitignored — run `buf generate` after cloning.
Adding an RPC to `ExecutorService` **breaks the build on purpose**:
`require_unimplemented_servers=false` means the server struct must implement it.
That tripwire is load-bearing; do not embed `UnimplementedExecutorServiceServer`
to silence it.
### Cross-platform notes
Development happens on Windows; the target is Linux. Two things bite:
- Go can `Listen` on an `AF_UNIX` socket on Windows but cannot `Dial` one. Socket
tests are build-tagged for Linux, with pipe-based equivalents running everywhere.
- Use `path`, not `filepath`, for anything describing a *container-side* path.
`filepath` follows host separator rules and will quietly pass `C:\evil` on Windows.
## Commits and pull requests
Conventional commits: `feat:`, `fix:`, `refactor:`, `docs:`, `test:`, `chore:`,
`perf:`, `ci:`.
A good PR body says what changed, why, and **what you measured**. "Tests pass" is
not a measurement. "Reverted the `path.Clean` check and `TestMountPathRejectsEscapeAttempts`
failed on two cases" is.
CI must be green before review: build for linux/amd64 and linux/arm64, `go vet`,
`golangci-lint` on both GOOS targets, unit tests with `-race`, the Electron build,
and the privileged-surface checks.
## Reporting bugs
Use the issue templates. For anything that crosses a privilege boundary, **do not
open an issue** — see [SECURITY.md](SECURITY.md).
## Scope
Kadran deliberately does not have a web panel. The management interface stays
behind SSH — the CLI today, the desktop app as it grows; adding a browser-facing
control surface would reintroduce exactly the attack surface the architecture removes. PRs adding one will be declined regardless of quality.
## License
Kadran is licensed under the [Apache License 2.0](LICENSE). Under its section 5, a
contribution you submit is licensed under the same terms unless you state
otherwise, and section 3 gives users a patent license for it. Do not submit code
you cannot license this way, for example code copied from a project under an
incompatible license.
If you bring in code from another Apache-2.0 project, keep its copyright notice
in the file and say where it came from in the pull request: its `NOTICE` text, if
it has one, must travel with it. Kadran has no `NOTICE` file of its own; add one
with that text.