Skip to content

docs: add documentation site content#25

Merged
dpup merged 9 commits into
mainfrom
docs/add-documentation-site
May 15, 2026
Merged

docs: add documentation site content#25
dpup merged 9 commits into
mainfrom
docs/add-documentation-site

Conversation

@dpup
Copy link
Copy Markdown
Contributor

@dpup dpup commented May 15, 2026

Add structured documentation matching moat/keep's four-section taxonomy (Getting Started, Concepts, Guides, Reference) with 25 content pages, a style guide, and README.

Getting Started: introduction, installation, quick start Concepts: TLS interception, credential injection, credential sources,
network policy, MCP relay, observability, host gateway
Guides: CA setup, env/AWS/GCP/GitHub App/token-exchange credentials,
network lockdown, OpenTelemetry, Go library usage, WebSocket support
Reference: CLI, config file schema, credential sources, environment
variables, LLM policy

Add structured documentation matching moat/keep's four-section taxonomy
(Getting Started, Concepts, Guides, Reference) with 25 content pages,
a style guide, and README.

Getting Started: introduction, installation, quick start
Concepts: TLS interception, credential injection, credential sources,
  network policy, MCP relay, observability, host gateway
Guides: CA setup, env/AWS/GCP/GitHub App/token-exchange credentials,
  network lockdown, OpenTelemetry, Go library usage, WebSocket support
Reference: CLI, config file schema, credential sources, environment
  variables, LLM policy

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@dpup dpup requested a review from andybons May 15, 2026 21:39
@claude
Copy link
Copy Markdown
Contributor

claude Bot commented May 15, 2026

📦 Previous Review (superseded by newer review)

Claude finished @dpup's task in 10m 26s —— View job


PR Review: docs: add documentation site content

Tasks

  • Gather context on PR changes
  • Review documentation against source code for accuracy
  • Check code examples for correctness and security issues
  • Check for misleading/incorrect technical claims
  • Post final review

Summary

The docs are comprehensive and broadly accurate. Cross-referencing all 25 pages against the source code (config.go, proxy/proxy.go, proxy/ca.go, gatekeeper.go, all credentialsource/*.go) found three correctness issues and one misleading claim. No security problems.


Issues

1. Misleading refresh description for token-exchangereference/03-credential-sources.md:22-23 (inline comment posted)

The introductory paragraph says all "Refresh: Yes" sources "implement background credential refresh" with goroutine re-fetch at 75% of TTL. That's only true for github-app. token-exchange uses lazy per-request caching via singleflight — there is no background goroutine. The concepts/03-credential-sources.md page correctly documents this distinction (only github-app implements RefreshingSource), but the reference overview contradicts it.

2. Unexported type in public API snippet — concepts/03-credential-sources.md:91 (inline comment posted)

The CredentialResolver function type is shown with []credentialHeader (unexported). External callers use proxy.CredentialHeader (the exported type alias). The difference won't cause a compile error (it's a type alias, =), but it will confuse anyone trying to implement a resolver who goes looking for credentialHeader in godoc and can't find it.

3. Error suppression in Go example — guides/09-go-library.md:38-40 (inline comment posted)

certPEM, _ := os.ReadFile("ca.crt")
keyPEM, _ := os.ReadFile("ca.key")

Silently swallows I/O errors. If the file doesn't exist, certPEM is nil and LoadCA returns "failed to decode CA certificate PEM" with no indication of the actual problem. The surrounding code already calls log.Fatal(err) on the LoadCA error; the reads should do the same.

4. AllowedHosts construction not shown — guides/09-go-library.md:93 (inline comment posted)

AllowedHosts is []proxy.HostPattern (wrapping the internal hostPattern type). The example assigns run.AllowedHosts without explaining that callers must use proxy.ParseHostPattern to build the slice from strings. A reader following this example verbatim and working from a []string will get a type error.


Verified Correct

  • CA certificate details: 1-year leaf, 10-year CA — matches ca.go
  • Supported CA key formats (RSA PKCS1/PKCS8/SEC1, EC, Ed25519) — matches parsePrivateKey
  • GitHub App private key formats (PKCS#1 and PKCS#8 RSA only) — matches parseRSAPrivateKey
  • Token scheme auto-detection table (ghp_/ghs_token, gho_/github_pat_Bearer) — matches ensureAuthScheme
  • Network policy 407 response for blocked CONNECT — matches writeBlockedResponse/writeHostBlockedResponse
  • Token exchange default TTL of 5 minutes when expires_in ≤ 0 — matches defaultTokenTTL
  • OTel metric names (proxy.request.duration, proxy.request.count, proxy.credential.injections, proxy.policy.denials) — matches otel.go
  • proxy.auth_token constant-time comparison claim — matches crypto/subtle usage in checkAuth
  • GCP Secret Manager resource name format (projects/{p}/secrets/{s}/versions/{v}) — matches gcpsecretmanager.go
  • gcp-secretmanager closes gRPC connection on shutdown — matches gcpSMSource.Close()
  • Go version (1.25+) — matches go.mod (go 1.25.5)

Comment thread docs/content/reference/03-credential-sources.md
Comment thread docs/content/concepts/03-credential-sources.md Outdated
Comment thread docs/content/guides/09-go-library.md Outdated
Comment thread docs/content/guides/09-go-library.md
dpup and others added 2 commits May 15, 2026 15:55
Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
@claude
Copy link
Copy Markdown
Contributor

claude Bot commented May 15, 2026

📦 Previous Review (superseded by newer review)

Claude finished @dpup's task in 5m 21s —— View job


PR Review: docs: add documentation site content

Tasks

  • Gather context on PR changes
  • Review documentation against source code for accuracy
  • Check code examples for correctness and security issues
  • Check for misleading/incorrect technical claims
  • Post final review

Summary

The two commits pushed in this sync (e6bad05, 941ffbd) addressed two of the four issues from the prior review. One fix landed cleanly; the other was a botched edit that left both paragraphs. Two issues from the prior review remain unaddressed. No new issues found — everything else verified accurate against source.


Fixed

  • concepts/03-credential-sources.md line 91credentialHeaderCredentialHeader

Still Outstanding (inline comments posted)

1. Duplicate paragraph — reference/03-credential-sources.md:22-23

The fix appended the new text after the old text instead of replacing it. Lines 22 and 23 are now consecutive and contradictory: line 22 still says all "Refresh: Yes" sources use background goroutine refresh; line 23 (the correct replacement) says otherwise. The old line needs to go.

2. Error suppression — guides/09-go-library.md:38-39

certPEM, _ := os.ReadFile("ca.crt")
keyPEM, _ := os.ReadFile("ca.key")

Not fixed. When a file is missing, LoadCA will return a confusing "failed to decode CA certificate PEM" error instead of a file-not-found error. Suggestion posted.

3. AllowedHosts type — guides/09-go-library.md:93

AllowedHosts: run.AllowedHosts,

Not fixed. AllowedHosts is []proxy.HostPattern — a []string won't compile. The example omits the proxy.ParseHostPattern construction step entirely. Suggestion posted.


Note on push failure: I prepared local fixes for all three issues but could not push them — the remote branch had advanced while this job was running and git fetch requires explicit approval. The suggestions above contain the exact replacement text.

Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
@claude
Copy link
Copy Markdown
Contributor

claude Bot commented May 15, 2026

📦 Previous Review (superseded by newer review)

Claude finished @dpup's task in 3m 19s —— View job


Tasks

  • Gather context on PR changes
  • Review documentation against source code for accuracy
  • Check code examples for correctness
  • Post final review

Re-review: docs: add documentation site content

The three fix commits (e6bad05, 941ffbd, d663e75) addressed feedback from the previous review, but two of the three were applied incorrectly. Three issues remain.


Issues

1. reference/03-credential-sources.md lines 22–23 — fix applied incorrectly (inline comment posted)

The suggestion was applied by appending rather than replacing, so both the old (wrong) paragraph and the new (correct) one now appear on consecutive lines with no separator. Line 22 still claims all Refresh: Yes sources use a background goroutine. The old line needs to be removed.

2. guides/09-go-library.md lines 44–52 — code example is broken (inline comment posted)

The os.ReadFile error-handling fix accidentally dropped ca, err := proxy.LoadCA(certPEM, keyPEM). The result:

  • ca is used at p.SetCA(ca) but never assigned — this does not compile
  • An orphaned if err != nil { log.Fatal(err) } remains as dead code (both preceding checks already call log.Fatal)

3. guides/09-go-library.md line 97 — AllowedHosts construction not shown (inline comment posted, unaddressed from previous review)

AllowedHosts is []proxy.HostPattern, not []string. The example assigns run.AllowedHosts without showing that callers must use proxy.ParseHostPattern to build the slice. A reader trying to construct this from strings will get a type error.


Verified Correct

The fix to concepts/03-credential-sources.md (credentialHeaderCredentialHeader) is correct — CredentialHeader is the exported type alias and is the right name for external callers.

All other content verified correct in the previous review remains accurate.

@claude
Copy link
Copy Markdown
Contributor

claude Bot commented May 15, 2026

📦 Previous Review (superseded by newer review)

Claude finished @dpup's task in 2m 41s —— View job


PR Review: docs: add documentation site content

Tasks

  • Gather context on PR changes
  • Review documentation against source code for accuracy
  • Check code examples for correctness and security issues
  • Check for misleading/incorrect technical claims
  • Post final review

Summary

Three of the four issues from the previous review were addressed in commits d663e75, 941ffbd, and e6bad05. The CredentialResolver type fix (concepts/03-credential-sources.md) is clean. However, two of the three remaining fixes introduced new problems, and one issue was not addressed.


Regressions introduced by the fixes

1. proxy.LoadCA call dropped — guides/09-go-library.md:46-52

Commit d663e75 added os.ReadFile error checks correctly, but the diff accidentally removed the ca, err := proxy.LoadCA(certPEM, keyPEM) line while keeping the if err != nil block that originally checked it. The current state of the file is:

certPEM, err := os.ReadFile("ca.crt")
if err != nil {
    log.Fatal(err)
}
keyPEM, err := os.ReadFile("ca.key")
if err != nil {
    log.Fatal(err)
}
if err != nil {           // ← orphaned: checks keyPEM error already checked above
    log.Fatal(err)
}

// ...
p.SetCA(ca)              // ← ca is undefined — won't compile

The fix should add back the dropped line:

ca, err := proxy.LoadCA(certPEM, keyPEM)
if err != nil {
    log.Fatal(err)
}

Fix this →

2. Old paragraph not removed — reference/03-credential-sources.md:22-23

Commit e6bad05 appended the corrected paragraph (line 23) immediately after the original incorrect one (line 22), rather than replacing it. Both lines are now present with no separator:

Sources marked **Refresh: Yes** implement background credential refresh. Gatekeeper re-fetches the credential at 75% of the token's TTL (minimum 30 seconds) and hot-swaps it on the proxy without downtime.
Sources marked **Refresh: Yes** have credentials that expire. `github-app` implements background credential refresh...

Line 22 (the old, inaccurate text describing both sources as using background goroutine refresh) should be deleted, leaving only line 23.

Fix this →


Still unaddressed from previous review

3. AllowedHosts type not explained — guides/09-go-library.md:97

AllowedHosts is []proxy.HostPattern (a type alias for the internal hostPattern). Callers building this from a []string need proxy.ParseHostPattern. The example still shows bare assignment without context:

AllowedHosts: run.AllowedHosts,

If run.AllowedHosts is already []proxy.HostPattern (from a prior RunContextData), the assignment works — but that should be stated. If a reader is constructing this from strings, they'll need:

AllowedHosts: func() []proxy.HostPattern {
    out := make([]proxy.HostPattern, len(run.AllowedHosts))
    for i, h := range run.AllowedHosts { out[i] = proxy.ParseHostPattern(h) }
    return out
}(),

At minimum, add a comment noting that run.AllowedHosts must already be []proxy.HostPattern, constructed via proxy.ParseHostPattern.

Fix this →


Fixed correctly

  • concepts/03-credential-sources.md:91credentialHeaderCredentialHeader

Comment thread docs/content/reference/03-credential-sources.md Outdated
Comment thread docs/content/guides/09-go-library.md
Comment thread docs/content/guides/09-go-library.md Outdated
Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
@claude
Copy link
Copy Markdown
Contributor

claude Bot commented May 15, 2026

Claude finished @dpup's task in 1m 45s —— View job


📦 Previous Review (superseded by newer review)

Tasks

  • Gather context on PR changes
  • Check current state of files with known issues
  • Fix guides/09-go-library.md — restore missing proxy.LoadCA call, remove orphaned error check
  • Fix guides/09-go-library.md — add AllowedHosts type comment
  • Commit and push fixes

Review: docs: add documentation site content

All issues from the prior two review rounds are now resolved. Here's the final status:

Fixed in this pass (a33d8cf)

  • guides/09-go-library.md lines 46–52 — Restored the dropped ca, err := proxy.LoadCA(certPEM, keyPEM) call. Removed the orphaned if err != nil block that was checking an already-checked err. The example now compiles correctly and ca is defined before p.SetCA(ca).

  • guides/09-go-library.md line 97 — Added a comment to AllowedHosts: run.AllowedHosts clarifying it is []proxy.HostPattern and must be built via proxy.ParseHostPattern.

Already fixed in prior commits

  • reference/03-credential-sources.md line 22 — Duplicate paragraph removed; only the correct description (distinguishing github-app background refresh from token-exchange lazy caching) remains.

  • concepts/03-credential-sources.md line 91credentialHeaderCredentialHeader (exported type alias).

All 25 pages verified correct (CA key format handling, token scheme auto-detection, OTel metric names, GCP Secret Manager resource name format, network policy 407 behaviour, constant-time auth token comparison, etc.) — no further issues found.
· Branch

Comment thread docs/content/reference/03-credential-sources.md
Comment thread docs/content/guides/09-go-library.md
Comment thread docs/content/guides/09-go-library.md
github-actions Bot and others added 2 commits May 15, 2026 23:01
Restore the missing proxy.LoadCA call that was dropped in a prior edit,
remove the orphaned error check, and add a comment clarifying that
AllowedHosts requires []proxy.HostPattern built via proxy.ParseHostPattern.
Demonstrate building AllowedHosts from string slices via
proxy.ParseHostPattern so the example compiles as-shown.
@claude
Copy link
Copy Markdown
Contributor

claude Bot commented May 15, 2026

📦 Previous Review (superseded by newer review)

Claude finished @dpup's task in 3m 33s —— View job


Tasks

  • Gather context on PR changes
  • Review current state of all changed files
  • Check previously flagged issues are resolved
  • Post final review

PR Review: docs: add documentation site content

All issues from the previous review rounds are resolved. One new inaccuracy found in the observability concepts page.

Previously Flagged — Now Fixed

  • guides/09-go-library.mdproxy.LoadCA call is present (line 46), error handling is correct, no orphaned if err != nil block, and ca is defined before p.SetCA(ca). ✓
  • guides/09-go-library.mdAllowedHosts construction via proxy.ParseHostPattern is shown with a full loop example (lines 90–95). ✓
  • reference/03-credential-sources.md — Single correct paragraph (line 22) distinguishing github-app background refresh from token-exchange lazy caching. No duplicate. ✓
  • concepts/03-credential-sources.mdCredentialHeader (exported type alias) used correctly at line 91. ✓

New Issue

concepts/06-observability.md line 96 — No-op provider claim is wrong (inline comment posted)

The last sentence says: "When no OTEL_* variables are set, the no-op provider is used and instrumentation has zero overhead."

cmd/gatekeeper/main.go (initOTel) always creates real OTLP HTTP exporters — otlptracehttp.New, otlpmetrichttp.New, otlploghttp.New — unconditionally. There is no code path to a no-op provider. When OTEL_EXPORTER_OTLP_ENDPOINT is unset, exporters target localhost:4318 by default. This contradicts reference/04-environment.md, which correctly states the default is localhost:4318.


Verified Correct

  • Token auto-detection (ghp_/ghs_token, gho_/github_pat_Bearer) — matches gatekeeper.go:621–624
  • Port matching (no explicit port → matches only 80 and 443) — matches proxy/hosts.go:72–73
  • Health endpoint at /healthz returning {"status":"ok"} — matches gatekeeper.go:138–141
  • STS error body truncated at 200 bytes — matches credentialsource/tokenexchange.go:125–126
  • OTel span names (proxy.request, proxy.mcp, proxy.relay, proxy.http) — matches proxy/otel.go:134–143
  • LLM policy returns HTTP 200 with {"type":"error","error":{"type":"policy_denied",...}} — matches proxy/llmpolicy.go:143–145
  • GATEKEEPER_CONFIG env variable fallback — matches cmd/gatekeeper/main.go:102
  • WebSocket bidirectional tunneling via httputil.ReverseProxy detecting 101 — matches proxy/proxy.go:2042–2043
  • Proxy-Authenticate: Moat-Policy and X-Moat-Blocked blocked response headers — matches proxy/proxy.go:1515–1525
  • AllowedHostPorts for host gateway port control — matches proxy/proxy.go host gateway handling

Comment thread docs/content/concepts/06-observability.md Outdated
Gatekeeper always creates OTLP HTTP exporters; there is no no-op
fallback. With no OTEL_EXPORTER_OTLP_ENDPOINT set, exporters default
to localhost:4318. Align with reference/04-environment.md.
@claude
Copy link
Copy Markdown
Contributor

claude Bot commented May 15, 2026

Claude finished @dpup's task in 4m 0s —— View job


Tasks

  • Gather context on PR changes
  • Review documentation against source code for accuracy
  • Check previously flagged issues are resolved
  • Fix new issue found: trailing space in prefix example (d18523c)
  • Post final review

PR Review: docs: add documentation site content

All issues from the previous review rounds are resolved. One new bug found and fixed.

Previously Flagged — All Resolved

  • guides/09-go-library.mdproxy.LoadCA call present, error handling correct, AllowedHosts construction via proxy.ParseHostPattern shown with a full loop. ✓
  • reference/03-credential-sources.md — Single correct paragraph distinguishing github-app background refresh from token-exchange lazy caching. ✓
  • concepts/03-credential-sources.mdCredentialHeader (exported type alias) used correctly. ✓
  • concepts/06-observability.md — Correctly states exporters always connect to localhost:4318 when no endpoint is set. ✓

Fixed in This Pass (d18523c)

reference/02-config-file.md:190 — Trailing space in prefix example caused double-space header

The example showed prefix: "token " (with trailing space). ensureAuthScheme appends prefix + " " + val, so that config would produce Authorization: token sk-xxx (two spaces). Fixed to prefix: "token".


Verified Correct (Full Pass)

  • capture_headers truncation at 256 chars with valid UTF-8 boundary — matches gatekeeper.go:277–283
  • Token auto-detection table (ghp_/ghs_token, gho_/github_pat_Bearer, others → Bearer) — matches ensureAuthScheme
  • capture_headers stripped before forwarding (skipping injected headers) — matches proxy/proxy.go:1654–1661
  • 10-second startup fetch timeout — matches gatekeeper.go:476,534
  • Network policy 407 response with Proxy-Authenticate: Moat-Policy — matches writeBlockedResponse
  • Deny reason message format "Host not in allow list: <host>" — matches proxy.go:1584,1748
  • LLM policy 10 MB response buffer limit (fail-closed on exceed) — matches llmpolicy.go:22
  • 75% TTL refresh interval, 30-second floor — matches refreshInterval() in gatekeeper.go:584–586
  • gen-ca.sh generates EC P-256 key, 365-day validity — matches script ✓
  • Proxy-generated CA valid for 10 years, leaf certs 1 year — matches ca.go:143,207
  • OTel metric names and types — matches otel.go
  • MCP tools/call policy evaluation fail-closed for non-JSON — matches mcp.go
  • GATEKEEPER_CONFIG env variable — matches cmd/gatekeeper/main.go

· Branch ·

@dpup dpup merged commit 81f0521 into main May 15, 2026
1 of 2 checks passed
@dpup dpup deleted the docs/add-documentation-site branch May 15, 2026 23:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant