diff --git a/README.md b/README.md index 9f0eea6..143f780 100644 --- a/README.md +++ b/README.md @@ -219,7 +219,7 @@ trace_flow(qualified_name: "webhook.WebhookHandler.ServeHTTP") ``` search(query: "authentication") → internal/adapters/inbound/webhook/handler.go (HMAC signature validation) -→ cmd/ccg-server/main.go (--webhook-secret flag) +→ cmd/ccg-server/main.go (CCG_WEBHOOK_SECRET / --webhook-secret) ``` ## MCP Server diff --git a/cmd/ccg-server/main.go b/cmd/ccg-server/main.go index 8b4e204..bbdaeef 100644 --- a/cmd/ccg-server/main.go +++ b/cmd/ccg-server/main.go @@ -141,7 +141,7 @@ func newRootCmd(rt *ccgruntime.Runtime, serviceVersion string) *cobra.Command { cmd.Flags().DurationVar(&cfg.WebhookRetryBaseDelay, "webhook-retry-base-delay", cfg.WebhookRetryBaseDelay, "Initial webhook sync retry delay") cmd.Flags().DurationVar(&cfg.WebhookRetryMaxDelay, "webhook-retry-max-delay", cfg.WebhookRetryMaxDelay, "Maximum webhook sync retry delay") cmd.Flags().StringSliceVar(&cfg.AllowRepo, "allow-repo", nil, "Allowed repo patterns for webhook sync (repeatable, e.g. org/*, !org/private)") - cmd.Flags().StringVar(&cfg.WebhookSecret, "webhook-secret", "", "HMAC secret for GitHub webhook signature verification") + cmd.Flags().StringVar(&cfg.WebhookSecret, "webhook-secret", cfg.WebhookSecret, "HMAC secret for GitHub webhook signature verification (env: CCG_WEBHOOK_SECRET)") cmd.Flags().BoolVar(&cfg.InsecureWebhook, "insecure-webhook", false, "Allow unsigned webhook requests (unsafe; testing only)") cmd.Flags().StringArrayVar(&cfg.RepoCloneBaseURLs, "repo-clone-base-url", nil, "Canonical base URL used to reconstruct clone targets for allowed repos (repeatable)") cmd.Flags().StringVar(&cfg.RepoRoot, "repo-root", cfg.RepoRoot, "Root directory for cloned repositories") diff --git a/cmd/ccg-server/main_test.go b/cmd/ccg-server/main_test.go new file mode 100644 index 0000000..9f185c9 --- /dev/null +++ b/cmd/ccg-server/main_test.go @@ -0,0 +1,33 @@ +package main + +import ( + "log/slog" + "testing" + + ccgruntime "github.com/tae2089/code-context-graph/internal/runtime" +) + +func TestWebhookSecretFlagUsesEnvironmentDefaultAndAllowsExplicitOverride(t *testing.T) { + t.Setenv("CCG_WEBHOOK_SECRET", "environment-test-value") + rt := ccgruntime.NewRuntime(slog.Default()) + defer rt.Close() + cmd := newRootCmd(rt, "test") + + got, err := cmd.Flags().GetString("webhook-secret") + if err != nil { + t.Fatalf("get webhook-secret flag: %v", err) + } + if got != "environment-test-value" { + t.Fatal("webhook-secret flag did not preserve the environment default") + } + if err := cmd.Flags().Set("webhook-secret", "explicit-test-value"); err != nil { + t.Fatalf("set webhook-secret flag: %v", err) + } + got, err = cmd.Flags().GetString("webhook-secret") + if err != nil { + t.Fatalf("get overridden webhook-secret flag: %v", err) + } + if got != "explicit-test-value" { + t.Fatal("explicit webhook-secret flag did not override the environment default") + } +} diff --git a/guide/cli-reference.md b/guide/cli-reference.md index 9900ca0..b2d5bff 100644 --- a/guide/cli-reference.md +++ b/guide/cli-reference.md @@ -141,7 +141,7 @@ HTTP MCP and webhook hosting now live in the dedicated `ccg-server` binary: | `ccg-server --wiki-dir ` | Enable the browser Wiki UI at `/wiki` using a built React dist directory; `/wiki/api/*` uses the same bearer token as `/mcp` | | `ccg-server --namespace-root ` | Root directory for file namespaces (default `namespaces`) | | `ccg-server --allow-repo ` | Allowed repo patterns for webhook sync (e.g. `org/*`, `org/api:main,develop`) | -| `ccg-server --webhook-secret ` | HMAC secret for webhook signature verification | +| `ccg-server --webhook-secret ` | HMAC secret for webhook signature verification (prefer `CCG_WEBHOOK_SECRET` to avoid argv exposure) | | `ccg-server --insecure-webhook` | Allow unsigned webhook requests for local testing only | | `ccg-server --repo-clone-base-url ` | Canonical base URL used to reconstruct webhook clone targets (repeatable) | | `ccg-server --repo-root ` | Root directory for cloned repositories | @@ -156,7 +156,7 @@ HTTP MCP and webhook hosting now live in the dedicated `ccg-server` binary: | `ccg-server --max-total-parsed-bytes ` | Maximum total bytes parsed across source files (`0` disables the limit) | Webhook-related server flags can also be configured with matching environment -variables where supported: `CCG_WEBHOOK_WORKERS`, +variables where supported: `CCG_WEBHOOK_SECRET`, `CCG_WEBHOOK_WORKERS`, `CCG_WEBHOOK_MAX_TRACKED_REPOS`, `CCG_WEBHOOK_ATTEMPT_TIMEOUT`, `CCG_WEBHOOK_RETRY_ATTEMPTS`, `CCG_WEBHOOK_RETRY_BASE_DELAY`, `CCG_WEBHOOK_RETRY_MAX_DELAY`, and `CCG_REPO_ROOT`. diff --git a/guide/docker.md b/guide/docker.md index 7df7258..bfad2e8 100644 --- a/guide/docker.md +++ b/guide/docker.md @@ -63,11 +63,11 @@ docker run -d -p 8080:8080 \ -e CCG_DB_DRIVER=postgres \ -e CCG_DB_DSN="$CCG_DB_DSN" \ -e CCG_REPO_ROOT=/data/repos \ + -e CCG_WEBHOOK_SECRET \ -v ccg-repos:/data/repos \ --entrypoint ccg-server ccg \ --http-addr :8080 \ --allow-repo "acme/*" \ - --webhook-secret "$WEBHOOK_SECRET" \ --repo-clone-base-url https://github.com ``` diff --git a/guide/ko/cli-reference.md b/guide/ko/cli-reference.md index 50112fb..d66c5e4 100644 --- a/guide/ko/cli-reference.md +++ b/guide/ko/cli-reference.md @@ -131,7 +131,7 @@ HTTP MCP와 웹훅 호스팅은 전용 `ccg-server` 바이너리에서 제공합 | `ccg-server --wiki-dir ` | 빌드된 React dist 디렉터리로 `/wiki` 브라우저 Wiki UI 활성화; `/wiki/api/*`는 `/mcp`와 같은 Bearer 토큰 사용 | | `ccg-server --namespace-root ` | 파일 네임스페이스의 루트 디렉토리 (기본값 `namespaces`) | | `ccg-server --allow-repo ` | 웹훅 동기화가 허용된 저장소 패턴 (예: `org/*`, `org/api:main,develop`) | -| `ccg-server --webhook-secret ` | 웹훅 서명 검증을 위한 HMAC 비밀키 | +| `ccg-server --webhook-secret ` | 웹훅 서명 검증을 위한 HMAC 비밀키(argv 노출 방지를 위해 `CCG_WEBHOOK_SECRET` 권장) | | `ccg-server --insecure-webhook` | 로컬 테스트 전용으로 서명되지 않은 웹훅 요청 허용 | | `ccg-server --repo-clone-base-url ` | 웹훅 복제 대상을 재구성하는 데 사용되는 정규 베이스 URL (반복 가능) | | `ccg-server --repo-root ` | 복제된 저장소의 루트 디렉토리 | @@ -145,7 +145,7 @@ HTTP MCP와 웹훅 호스팅은 전용 `ccg-server` 바이너리에서 제공합 | `ccg-server --max-file-bytes ` | 파싱된 소스 파일당 허용되는 최대 바이트 수 (`0`은 제한 없음) | | `ccg-server --max-total-parsed-bytes ` | 소스 파일 전체에서 파싱된 최대 총 바이트 수 (`0`은 제한 없음) | -웹훅 관련 server 플래그는 지원되는 경우 일치하는 환경 변수로도 설정할 수 있습니다: `CCG_WEBHOOK_WORKERS`, `CCG_WEBHOOK_MAX_TRACKED_REPOS`, `CCG_WEBHOOK_ATTEMPT_TIMEOUT`, `CCG_WEBHOOK_RETRY_ATTEMPTS`, `CCG_WEBHOOK_RETRY_BASE_DELAY`, `CCG_WEBHOOK_RETRY_MAX_DELAY`, `CCG_REPO_ROOT`. +웹훅 관련 server 플래그는 지원되는 경우 일치하는 환경 변수로도 설정할 수 있습니다: `CCG_WEBHOOK_SECRET`, `CCG_WEBHOOK_WORKERS`, `CCG_WEBHOOK_MAX_TRACKED_REPOS`, `CCG_WEBHOOK_ATTEMPT_TIMEOUT`, `CCG_WEBHOOK_RETRY_ATTEMPTS`, `CCG_WEBHOOK_RETRY_BASE_DELAY`, `CCG_WEBHOOK_RETRY_MAX_DELAY`, `CCG_REPO_ROOT`. `CCG_HTTP_BEARER_TOKEN`은 `--http-bearer-token`에 대해서도 지원되며, `CCG_OTEL_ENDPOINT`는 `--otel-endpoint`에 대해서도 지원됩니다. 이 토큰은 `/mcp`의 MCP HTTP 엔드포인트를 보호하지만, `/health`, `/ready`, `/status`, `/webhook` 자체를 비공개로 만들지는 않습니다. diff --git a/guide/ko/docker.md b/guide/ko/docker.md index 60a2528..652e8a7 100644 --- a/guide/ko/docker.md +++ b/guide/ko/docker.md @@ -47,11 +47,11 @@ docker run -d -p 8080:8080 \ -e CCG_DB_DRIVER=postgres \ -e CCG_DB_DSN="$CCG_DB_DSN" \ -e CCG_REPO_ROOT=/data/repos \ + -e CCG_WEBHOOK_SECRET \ -v ccg-repos:/data/repos \ --entrypoint ccg-server ccg \ --http-addr :8080 \ --allow-repo "acme/*" \ - --webhook-secret "$WEBHOOK_SECRET" \ --repo-clone-base-url https://github.com ``` diff --git a/guide/ko/operations.md b/guide/ko/operations.md index fca141d..2bb131e 100644 --- a/guide/ko/operations.md +++ b/guide/ko/operations.md @@ -159,12 +159,12 @@ CCG가 Ingress, 리버스 프록시 또는 로드 밸런서 뒤에 있는 경우 웹훅 배포는 다음 사항들을 설정해야 합니다: - 명시적인 저장소 및 브랜치 허용 목록을 위한 `--allow-repo` -- HMAC 검증을 위한 `--webhook-secret` +- HMAC 검증을 위한 `CCG_WEBHOOK_SECRET`(권장) 또는 `--webhook-secret` - 복제 URL이 페이로드로부터 신뢰받는 대신 허용된 저장소 이름으로부터 재구성되도록 하는 `--repo-clone-base-url` - 지속성 있는 로컬 스토리지를 위한 `--repo-root` - 팀용 또는 상시 가동 배포를 위한 `--db-driver postgres` -웹훅 네임스페이스 추출은 마지막 저장소 이름을 사용합니다. 예를 들어 `acme/api`는 `api` 네임스페이스에 저장되고 `$REPO_ROOT/api`에 checkout됩니다. 이 전략은 단일 owner 웹훅 배포를 위한 것입니다. 허용 목록이 여러 owner를 포함하면 서버는 경고를 남깁니다. `acme/api`와 `external/api`가 같은 네임스페이스 및 checkout 경로에서 충돌할 수 있기 때문입니다. +웹훅 네임스페이스 추출은 마지막 저장소 이름을 사용합니다. 예를 들어 `acme/api`는 `api` 네임스페이스에 저장되고 `$REPO_ROOT/api`에 checkout됩니다. 이 전략은 단일 owner 웹훅 배포를 위한 것입니다. 허용 목록이 여러 owner를 포함하면 `acme/api`와 `external/api`가 같은 네임스페이스 및 checkout 경로에서 충돌할 수 있으므로 서버는 시작을 거부합니다. 권장 정책: @@ -253,7 +253,7 @@ CCG를 업그레이드한 후, 기존의 기본 `ccg.db` 또한 기존 스키마 | 증상 | 가능성 높은 원인 | 확인 / 해결 방법 | |---------|--------------|-------------| | `401` 또는 MCP 초기화 실패 | Bearer 토큰 누락 또는 오류 | `Authorization: Bearer ...` 및 `CCG_HTTP_BEARER_TOKEN`을 확인하십시오. | -| 웹훅이 권한 없음(unauthorized) 반환 | HMAC 서명 누락/유효하지 않음 | `--webhook-secret` 및 제공자의 서명 헤더를 확인하십시오. | +| 웹훅이 권한 없음(unauthorized) 반환 | HMAC 서명 누락/유효하지 않음 | `CCG_WEBHOOK_SECRET`(또는 `--webhook-secret`) 및 제공자의 서명 헤더를 확인하십시오. | | 웹훅이 금지됨(forbidden) 반환 | 허용되지 않은 저장소 또는 브랜치 | `--allow-repo` 패턴 및 브랜치 ref를 확인하십시오. | | 웹훅이 너무 많은 요청 반환 | 동기화 큐가 가득 참 | `/status`를 확인하고, push 볼륨을 줄이거나 PostgreSQL에서 워커를 늘리십시오. | | `/ready`가 `not_ready`임 | DB 또는 큐 차단 조건 | `/status` 및 서비스 로그를 조사하십시오. | diff --git a/guide/ko/runtime-layout.md b/guide/ko/runtime-layout.md index 0130deb..5326346 100644 --- a/guide/ko/runtime-layout.md +++ b/guide/ko/runtime-layout.md @@ -83,12 +83,13 @@ ccg-server --http-addr 0.0.0.0:8080 --http-bearer-token "$CCG_HTTP_BEARER_TOKEN" Webhook sync: +이 명령을 시작하기 전에 배포 secret store를 통해 `CCG_WEBHOOK_SECRET`을 설정하십시오. + ```bash ccg-server \ --http-addr 0.0.0.0:8080 \ --http-bearer-token "$CCG_HTTP_BEARER_TOKEN" \ --allow-repo "org/api:main,develop" \ - --webhook-secret "$WEBHOOK_SECRET" \ --repo-clone-base-url https://github.com \ --repo-root /data/repos ``` diff --git a/guide/ko/webhook.md b/guide/ko/webhook.md index bb822d7..a8c0466 100644 --- a/guide/ko/webhook.md +++ b/guide/ko/webhook.md @@ -6,11 +6,12 @@ GitHub 또는 Gitea로부터 push 이벤트를 수신하여 자동으로 복제( ## 설정 (Setup) +서버 시작 전에 배포 환경의 secret store를 통해 `CCG_WEBHOOK_SECRET`을 설정하십시오. 환경 변수 기반 설정은 값을 프로세스 인자에 노출하지 않습니다. + ```bash ccg-server \ --allow-repo "org/api:main,develop" \ --allow-repo "org/web:main" \ - --webhook-secret "your-secret" \ --repo-clone-base-url https://github.com \ --repo-root /data/repos ``` @@ -80,7 +81,7 @@ ccg-server \ 웹훅 동기화는 `org/repo` 중 마지막 저장소 이름을 그래프 네임스페이스와 checkout 디렉터리로 사용합니다. 예를 들어 `acme/api`는 `api` 네임스페이스와 `/data/repos/api` checkout으로 매핑됩니다. -이 방식은 단일 조직 배포에서 짧고 예측 가능한 이름을 유지하기 위한 전략이며, 권장 운영 모델입니다. 허용 목록이 `acme/*`와 `external/shared`처럼 여러 owner를 포함하거나 `*/*`를 사용하면, 같은 마지막 저장소 이름이 충돌할 수 있으므로 CCG는 시작 시 경고 로그를 남깁니다. +이 방식은 단일 조직 배포에서 짧고 예측 가능한 이름을 유지하기 위한 전략이며, 필수 운영 모델입니다. 허용 목록이 `acme/*`와 `external/shared`처럼 여러 owner를 포함하거나 `*/*`를 사용하면 같은 마지막 저장소 이름이 충돌할 수 있으므로 CCG는 시작을 거부합니다. | 저장소 | 파생 네임스페이스 | |------|-------------------| @@ -89,7 +90,7 @@ ccg-server \ 운영 정책: -- 웹훅 CCG 인스턴스 하나에는 하나의 owner/조직을 사용하는 것을 권장합니다. +- 웹훅 CCG 인스턴스 하나에는 하나의 owner/조직만 사용하십시오. 여러 owner 또는 wildcard owner는 시작 시 거부됩니다. - 같은 인스턴스 안에 마지막 저장소 이름이 같은 저장소를 허용하지 마십시오. - 여러 owner의 동기화가 필요하면 별도 CCG 인스턴스를 사용하거나, 네임스페이스 전략을 변경한 뒤 활성화하십시오. @@ -102,9 +103,9 @@ HMAC-SHA256을 사용하여 웹훅 페이로드를 검증합니다. | GitHub | `X-Hub-Signature-256` | `sha256=` | | Gitea | `X-Gitea-Signature` | `` | -기본적으로 `--webhook-secret`이 설정되지 않으면 웹훅 요청은 실패(fail closed) 처리됩니다. +기본적으로 `CCG_WEBHOOK_SECRET` 또는 `--webhook-secret`이 설정되지 않으면 웹훅 요청은 실패(fail closed) 처리됩니다. -- `--webhook-secret`은 HMAC 검증을 활성화합니다. +- `CCG_WEBHOOK_SECRET`(권장) 또는 `--webhook-secret`은 HMAC 검증을 활성화합니다. 환경 변수를 사용하면 프로세스 인자에 값이 노출되지 않습니다. - `--insecure-webhook`은 명시적인 테스트 전용 옵션이며 `--webhook-secret`과 함께 사용할 수 없습니다. - 보안 모드에서 실행할 때는 `--repo-clone-base-url`이 필수이며, 서버는 웹훅 페이로드의 `clone_url`을 신뢰하는 대신 허용된 저장소 이름을 기반으로 복제 URL을 재구성합니다. diff --git a/guide/operations.md b/guide/operations.md index c0e30db..8fc213c 100644 --- a/guide/operations.md +++ b/guide/operations.md @@ -196,7 +196,7 @@ can cover request intake, queue processing, clone/pull, and graph update work. Webhook deployments should be configured with: - `--allow-repo` for an explicit repository and branch allowlist -- `--webhook-secret` for HMAC verification +- `CCG_WEBHOOK_SECRET` (preferred) or `--webhook-secret` for HMAC verification - `--repo-clone-base-url` so clone URLs are reconstructed from allowed repo names instead of trusted from payloads - `--repo-root` on durable local storage @@ -205,7 +205,7 @@ Webhook deployments should be configured with: Webhook namespace extraction uses the final repository name. For example, `acme/api` is stored in namespace `api` and checked out under `$REPO_ROOT/api`. This is intended for single-owner webhook deployments. If an -allowlist spans multiple owners, the server logs a warning because `acme/api` +allowlist spans multiple owners, the server rejects startup because `acme/api` and `external/api` would collide in the same namespace and checkout path. Recommended policy: @@ -333,7 +333,7 @@ existing schema and migrated explicitly before restarting runtime commands. | Symptom | Likely Cause | Check / Fix | |---------|--------------|-------------| | `401` or MCP initialize fails | Missing or wrong bearer token | Confirm `Authorization: Bearer ...` and `CCG_HTTP_BEARER_TOKEN`. | -| Webhook returns unauthorized | Missing/invalid HMAC signature | Verify `--webhook-secret` and provider signature header. | +| Webhook returns unauthorized | Missing/invalid HMAC signature | Verify `CCG_WEBHOOK_SECRET` (or `--webhook-secret`) and the provider signature header. | | Webhook returns forbidden | Repo or branch not allowed | Check `--allow-repo` patterns and branch refs. | | Webhook returns too many requests | Sync queue is full | Check `/status`, reduce push volume, or increase workers on PostgreSQL. | | `/ready` is `not_ready` | DB or queue blocking condition | Inspect `/status` and service logs. | diff --git a/guide/runtime-layout.md b/guide/runtime-layout.md index 7fe12dc..44d2bd5 100644 --- a/guide/runtime-layout.md +++ b/guide/runtime-layout.md @@ -86,12 +86,13 @@ ccg-server \ Webhook sync: +Set `CCG_WEBHOOK_SECRET` through the deployment secret store before starting this command. + ```bash ccg-server \ --http-addr 0.0.0.0:8080 \ --http-bearer-token "$CCG_HTTP_BEARER_TOKEN" \ --allow-repo "org/api:main,develop" \ - --webhook-secret "$WEBHOOK_SECRET" \ --repo-clone-base-url https://github.com \ --repo-root /data/repos ``` diff --git a/guide/webhook.md b/guide/webhook.md index 1a251a6..1c634e2 100644 --- a/guide/webhook.md +++ b/guide/webhook.md @@ -4,11 +4,12 @@ Receives push events from GitHub or Gitea and automatically performs clone/pull ## Setup +Set `CCG_WEBHOOK_SECRET` through your deployment's secret store before starting the server. The environment-backed setting keeps the value out of process arguments. + ```bash ccg-server \ --allow-repo "org/api:main,develop" \ --allow-repo "org/web:main" \ - --webhook-secret "your-secret" \ --repo-clone-base-url https://github.com \ --repo-root /data/repos ``` @@ -89,8 +90,8 @@ repository name portion of `org/repo`. For example, `acme/api` maps to the `api` namespace and `/data/repos/api` checkout. This keeps single-organization deployments short and predictable, which is the -recommended operating model. If the allowlist spans multiple owners, such as -`acme/*` plus `external/shared` or `*/*`, CCG logs a startup warning because +required operating model. If the allowlist spans multiple owners, such as +`acme/*` plus `external/shared` or `*/*`, CCG rejects startup because repositories with the same final name can collide: | Repo | Derived namespace | @@ -100,7 +101,7 @@ repositories with the same final name can collide: Operational policy: -- Prefer one owner/organization per webhook CCG instance. +- Use one owner/organization per webhook CCG instance; startup rejects multiple or wildcard owners. - Do not allow two repositories with the same final repo name in the same instance. - If multi-owner sync becomes necessary, run separate CCG instances or change @@ -115,9 +116,9 @@ Verifies webhook payload with HMAC-SHA256. | GitHub | `X-Hub-Signature-256` | `sha256=` | | Gitea | `X-Gitea-Signature` | `` | -By default, webhook requests fail closed unless `--webhook-secret` is configured. +By default, webhook requests fail closed unless `CCG_WEBHOOK_SECRET` or `--webhook-secret` is configured. -- `--webhook-secret` enables HMAC verification. +- `CCG_WEBHOOK_SECRET` (preferred) or `--webhook-secret` enables HMAC verification. The environment variable avoids exposing the value in process arguments. - `--insecure-webhook` is an explicit testing-only escape hatch and is mutually exclusive with `--webhook-secret`. - When running in secure mode, `--repo-clone-base-url` is required and the server reconstructs clone URLs from the allowed repository name instead of trusting `clone_url` from the webhook payload. diff --git a/internal/adapters/inbound/http/config.go b/internal/adapters/inbound/http/config.go index 01f7d71..1469638 100644 --- a/internal/adapters/inbound/http/config.go +++ b/internal/adapters/inbound/http/config.go @@ -7,6 +7,8 @@ import ( "strconv" "strings" "time" + + "github.com/tae2089/code-context-graph/internal/app/reposync" ) // Config holds self-hosted HTTP server runtime options. @@ -52,6 +54,7 @@ func DefaultConfig() Config { OTELEndpoint: os.Getenv("CCG_OTEL_ENDPOINT"), WikiDir: os.Getenv("CCG_WIKI_DIR"), NamespaceRoot: "namespaces", + WebhookSecret: os.Getenv("CCG_WEBHOOK_SECRET"), WebhookWorkers: EnvInt("CCG_WEBHOOK_WORKERS", 4), WebhookMaxTrackedRepos: EnvInt("CCG_WEBHOOK_MAX_TRACKED_REPOS", 1024), WebhookAttemptTimeout: EnvDuration("CCG_WEBHOOK_ATTEMPT_TIMEOUT", 15*time.Minute), @@ -98,6 +101,13 @@ func ValidateConfig(cfg Config) error { if len(cfg.AllowRepo) == 0 { return nil } + rules := make([]reposync.RepoRule, 0, len(cfg.AllowRepo)) + for _, raw := range cfg.AllowRepo { + rules = append(rules, reposync.ParseRepoRule(raw)) + } + if err := reposync.ValidateRepoNameNamespaceRules(rules); err != nil { + return err + } if cfg.WebhookSecret != "" && cfg.InsecureWebhook { return fmt.Errorf("--webhook-secret and --insecure-webhook are mutually exclusive") } diff --git a/internal/adapters/inbound/http/config_test.go b/internal/adapters/inbound/http/config_test.go index 6ffde40..13ec185 100644 --- a/internal/adapters/inbound/http/config_test.go +++ b/internal/adapters/inbound/http/config_test.go @@ -44,6 +44,20 @@ func TestValidateConfig_WebhookAllowsSecureModeWithCloneBaseURL(t *testing.T) { } } +func TestValidateConfig_RejectsMultiOwnerRepoNameNamespaces(t *testing.T) { + cfg := DefaultConfig() + cfg.Transport = "streamable-http" + cfg.AllowRepo = []string{"org-a/*", "org-b/api"} + cfg.WebhookSecret = "test-placeholder" + cfg.RepoRoot = "/var/repos" + cfg.RepoCloneBaseURLs = []string{"https://github.com"} + + err := ValidateConfig(cfg) + if err == nil || !strings.Contains(err.Error(), "repo-name namespaces") { + t.Fatalf("expected repo-name namespace validation error, got %v", err) + } +} + func TestConfiguredCloneBaseURLs_PreservesLegacySingularFirst(t *testing.T) { cfg := Config{ RepoCloneBaseURL: "https://github.com", @@ -103,3 +117,12 @@ func TestDefaultConfig_UsesWebhookTuningFromEnv(t *testing.T) { t.Fatalf("unexpected webhook env config: %+v", cfg) } } + +func TestDefaultConfig_UsesWebhookSecretFromEnv(t *testing.T) { + t.Setenv("CCG_WEBHOOK_SECRET", "configured-for-test") + + cfg := DefaultConfig() + if cfg.WebhookSecret != "configured-for-test" { + t.Fatal("DefaultConfig did not load CCG_WEBHOOK_SECRET") + } +} diff --git a/internal/adapters/outbound/contentfiles/wiki.go b/internal/adapters/outbound/contentfiles/wiki.go index 42d9101..0feab78 100644 --- a/internal/adapters/outbound/contentfiles/wiki.go +++ b/internal/adapters/outbound/contentfiles/wiki.go @@ -11,6 +11,7 @@ import ( "github.com/tae2089/code-context-graph/internal/app/wiki" requestctx "github.com/tae2089/code-context-graph/internal/ctx" + "github.com/tae2089/code-context-graph/internal/safepath" ) // WikiIndexWriter maps namespaces to compatibility snapshot paths and atomically replaces JSON. @@ -23,23 +24,29 @@ var _ wiki.IndexWriter = (*WikiIndexWriter)(nil) // @intent preserve the default .ccg output root while allowing CLI-configured state paths. func NewWikiIndexWriter(root string) *WikiIndexWriter { return &WikiIndexWriter{Root: root} } -// @intent map default and named namespaces to their compatibility snapshot location. -func (w *WikiIndexWriter) indexPath(namespace string) string { +// @intent map default and validated single-segment namespaces to their compatibility snapshot location. +func (w *WikiIndexWriter) indexPath(namespace string) (string, error) { root := w.Root if root == "" { root = ".ccg" } if requestctx.Normalize(namespace) == requestctx.DefaultNamespace { - return filepath.Join(root, "wiki-index.json") + return filepath.Join(root, "wiki-index.json"), nil } - return filepath.Join(root, namespace, "wiki-index.json") + if err := safepath.ValidateNamespacePath(namespace, ""); err != nil { + return "", err + } + return filepath.Join(root, namespace, "wiki-index.json"), nil } // WriteWikiIndex writes indented JSON through a same-directory temporary file and rename. // @intent preserve the versioned built-in Wiki snapshot format at its namespace-specific path. // @sideEffect creates the namespace directory and atomically replaces wiki-index.json. func (w *WikiIndexWriter) WriteWikiIndex(_ context.Context, namespace string, idx *wiki.Index) error { - target := w.indexPath(namespace) + target, err := w.indexPath(namespace) + if err != nil { + return trace.Wrap(err, "validate wiki index namespace") + } dir := filepath.Dir(target) if err := os.MkdirAll(dir, 0o755); err != nil { return trace.Wrap(err, "mkdir wiki index dir") diff --git a/internal/adapters/outbound/contentfiles/wiki_test.go b/internal/adapters/outbound/contentfiles/wiki_test.go new file mode 100644 index 0000000..85ec718 --- /dev/null +++ b/internal/adapters/outbound/contentfiles/wiki_test.go @@ -0,0 +1,47 @@ +package contentfiles + +import ( + "context" + "os" + "path/filepath" + "testing" + + "github.com/tae2089/code-context-graph/internal/app/wiki" +) + +func TestWikiIndexWriterRejectsUnsafeNamespaceBeforeWriting(t *testing.T) { + outer := t.TempDir() + root := filepath.Join(outer, "root") + writer := NewWikiIndexWriter(root) + + tests := []struct { + name string + namespace string + }{ + {name: "parent traversal", namespace: "../escape"}, + {name: "nested namespace", namespace: "owner/repo"}, + {name: "absolute namespace", namespace: filepath.Join(string(filepath.Separator), "escape")}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if err := writer.WriteWikiIndex(context.Background(), tt.namespace, &wiki.Index{}); err == nil { + t.Fatalf("WriteWikiIndex(%q) = nil, want validation error", tt.namespace) + } + }) + } + + if _, err := os.Stat(filepath.Join(outer, "escape", "wiki-index.json")); !os.IsNotExist(err) { + t.Fatalf("unsafe namespace wrote outside root: %v", err) + } +} + +func TestWikiIndexWriterPreservesDefaultNamespace(t *testing.T) { + root := t.TempDir() + writer := NewWikiIndexWriter(root) + if err := writer.WriteWikiIndex(context.Background(), "", &wiki.Index{}); err != nil { + t.Fatalf("WriteWikiIndex(default): %v", err) + } + if _, err := os.Stat(filepath.Join(root, "wiki-index.json")); err != nil { + t.Fatalf("default wiki index: %v", err) + } +} diff --git a/internal/app/docs/generator.go b/internal/app/docs/generator.go index a5239c7..86df7fe 100644 --- a/internal/app/docs/generator.go +++ b/internal/app/docs/generator.go @@ -57,7 +57,10 @@ func (g *Generator) Run() error { current := generatedFiles(groups) var errs []error - previous, _ := g.loadManifest() + previous, err := g.loadManifest() + if err != nil { + return fmt.Errorf("load docs manifest: %w", err) + } for _, grp := range groups { if err := g.writeFileDoc(grp); err != nil { errs = append(errs, fmt.Errorf("write file doc %s: %w", grp.FilePath, err)) diff --git a/internal/app/docs/generator_test.go b/internal/app/docs/generator_test.go index 696ccf5..f63b442 100644 --- a/internal/app/docs/generator_test.go +++ b/internal/app/docs/generator_test.go @@ -449,3 +449,45 @@ func TestRun_PruneDeletesOnlyGeneratorManagedStaleDocs(t *testing.T) { t.Fatalf("user doc must not be pruned: %v", err) } } + +func TestRun_MalformedManifestFailsBeforeMutatingOutputs(t *testing.T) { + db := newTestDB(t) + node := graph.Node{QualifiedName: "pkg.New", Kind: graph.NodeKindFunction, Name: "New", FilePath: "pkg/new.go", StartLine: 1, EndLine: 5, Hash: "h1", Language: "go"} + if err := db.Create(&node).Error; err != nil { + t.Fatalf("create node: %v", err) + } + + gen, outDir := newGenerator(t, db) + gen.Prune = true + manifestPath := filepath.Join(outDir, ".ccg-docs-manifest.json") + malformed := []byte("{not-json") + if err := os.WriteFile(manifestPath, malformed, 0o644); err != nil { + t.Fatalf("write malformed manifest: %v", err) + } + indexPath := filepath.Join(outDir, "index.md") + indexBefore := []byte("operator-owned sentinel\n") + if err := os.WriteFile(indexPath, indexBefore, 0o644); err != nil { + t.Fatalf("write sentinel index: %v", err) + } + + if err := gen.Run(); err == nil { + t.Fatal("Run() = nil, want malformed manifest error") + } + manifestAfter, err := os.ReadFile(manifestPath) + if err != nil { + t.Fatalf("read manifest after failed run: %v", err) + } + if string(manifestAfter) != string(malformed) { + t.Fatalf("manifest changed after failed load: %q", manifestAfter) + } + indexAfter, err := os.ReadFile(indexPath) + if err != nil { + t.Fatalf("read index after failed run: %v", err) + } + if string(indexAfter) != string(indexBefore) { + t.Fatalf("index changed after failed manifest load: %q", indexAfter) + } + if _, err := os.Stat(filepath.Join(outDir, "pkg", "new.go.md")); !os.IsNotExist(err) { + t.Fatalf("file doc written after failed manifest load: %v", err) + } +} diff --git a/internal/app/ingest/workflow/build.go b/internal/app/ingest/workflow/build.go index a9add6c..9b178f6 100644 --- a/internal/app/ingest/workflow/build.go +++ b/internal/app/ingest/workflow/build.go @@ -296,7 +296,7 @@ func (s *Service) collectBuildParseInputs(ctx context.Context, absDir string, op return nil } - info, err := os.Stat(path) + info, err := inspectRegularSourceFile(path) if err != nil { return trace.Wrap(err, "stat build file "+relPath) } @@ -441,7 +441,7 @@ func (s *Service) parseBuildInput(ctx context.Context, input buildParseInput) bu result.err = err return result } - content, err := os.ReadFile(input.path) + content, err := readRegularSourceFile(input.path) if err != nil { result.err = trace.Wrap(err, "read build file "+input.relPath) return result diff --git a/internal/app/ingest/workflow/fileio.go b/internal/app/ingest/workflow/fileio.go index 69e202c..31ad8cb 100644 --- a/internal/app/ingest/workflow/fileio.go +++ b/internal/app/ingest/workflow/fileio.go @@ -4,6 +4,7 @@ package workflow import ( "context" "fmt" + "io" "log/slog" "os" "path/filepath" @@ -15,6 +16,63 @@ import ( "github.com/tae2089/code-context-graph/internal/pathspec" ) +// inspectRegularSourceFile reads source metadata without following symlinks. +// @intent reject symlink and non-regular source paths before any parser or package discoverer can read target bytes. +func inspectRegularSourceFile(path string) (os.FileInfo, error) { + info, err := os.Lstat(path) + if err != nil { + return nil, err + } + if info.Mode()&os.ModeSymlink != 0 { + return nil, fmt.Errorf("source symlink is not allowed: %s", path) + } + if !info.Mode().IsRegular() { + return nil, fmt.Errorf("source path is not a regular file: %s", path) + } + return info, nil +} + +// openRegularSourceFile opens a source only when its no-follow identity remains stable across inspection and open. +// @intent prevent replacement races from turning a validated regular path into a followed symlink before reading. +func openRegularSourceFile(path string) (*os.File, os.FileInfo, error) { + before, err := inspectRegularSourceFile(path) + if err != nil { + return nil, nil, err + } + file, err := os.Open(path) + if err != nil { + return nil, nil, err + } + after, err := file.Stat() + if err != nil { + _ = file.Close() + return nil, nil, err + } + if !after.Mode().IsRegular() || !os.SameFile(before, after) { + _ = file.Close() + return nil, nil, fmt.Errorf("source file changed while opening: %s", path) + } + return file, after, nil +} + +// readRegularSourceFile reads bytes from a verified regular-file descriptor. +// @intent keep all secondary source reads on the same no-follow path as build and update ingestion. +func readRegularSourceFile(path string) ([]byte, error) { + file, _, err := openRegularSourceFile(path) + if err != nil { + return nil, err + } + content, readErr := io.ReadAll(file) + closeErr := file.Close() + if readErr != nil { + return nil, readErr + } + if closeErr != nil { + return nil, closeErr + } + return content, nil +} + // shouldSkipDir reports whether the source walker must skip a directory name. // @intent keep default source traversal exclusions local to the ingest workflow. // @domainRule .git, vendor, node_modules, and hidden directories except . are skipped. diff --git a/internal/app/ingest/workflow/indexer_test.go b/internal/app/ingest/workflow/indexer_test.go index 31e1f83..22d6feb 100644 --- a/internal/app/ingest/workflow/indexer_test.go +++ b/internal/app/ingest/workflow/indexer_test.go @@ -3346,6 +3346,42 @@ func TestBuild_ReadFailure_PreservesPreviousGraphState(t *testing.T) { assertFunctionNamesByFile(t, st, ctx, "sample.go", []string{"Keep"}) } +func TestBuild_RejectsLiveSourceSymlinkOutsideRoot(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("source symlink scenario is unix-specific") + } + + db, err := gorm.Open(sqlite.Open(":memory:"), &gorm.Config{Logger: gormlogger.Discard}) + if err != nil { + t.Fatalf("open db: %v", err) + } + st := graphgorm.New(db) + if err := st.AutoMigrate(); err != nil { + t.Fatalf("migrate: %v", err) + } + svc := &Service{ + Store: st, + UnitOfWork: newTestUnitOfWork(db, nil), + Walkers: map[string]Parser{".go": treesitter.NewWalker(treesitter.GoSpec)}, + Logger: slog.Default(), + } + + root := t.TempDir() + targetDir := t.TempDir() + target := filepath.Join(targetDir, "outside.go") + if err := os.WriteFile(target, []byte("package outside\n\nfunc Leaked() {}\n"), 0o644); err != nil { + t.Fatalf("write outside target: %v", err) + } + if err := os.Symlink(target, filepath.Join(root, "linked.go")); err != nil { + t.Fatalf("create live symlink: %v", err) + } + + if _, err := svc.Build(context.Background(), BuildOptions{Dir: root}); err == nil { + t.Fatal("expected Build to reject a live source symlink") + } + assertFunctionNamesByFile(t, st, context.Background(), "linked.go", nil) +} + func TestBuild_MissingRoot_DoesNotDeleteExistingGraph(t *testing.T) { db, err := gorm.Open(sqlite.Open(":memory:"), &gorm.Config{Logger: gormlogger.Discard}) if err != nil { @@ -3556,6 +3592,56 @@ func TestUpdate_SkipsUnreadableFiles(t *testing.T) { } } +func TestUpdate_SourceSymlinkPreservesPreviouslyIndexedFile(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("source symlink scenario is unix-specific") + } + + db, err := gorm.Open(sqlite.Open(":memory:"), &gorm.Config{Logger: gormlogger.Discard}) + if err != nil { + t.Fatalf("open db: %v", err) + } + st := graphgorm.New(db) + if err := st.AutoMigrate(); err != nil { + t.Fatalf("migrate: %v", err) + } + walker := treesitter.NewWalker(treesitter.GoSpec) + svc := &Service{ + Store: st, + UnitOfWork: newTestUnitOfWork(db, nil), + Walkers: map[string]Parser{".go": walker}, + Logger: slog.Default(), + } + + root := t.TempDir() + sourcePath := filepath.Join(root, "sample.go") + if err := os.WriteFile(sourcePath, []byte("package sample\n\nfunc Keep() {}\n"), 0o644); err != nil { + t.Fatalf("write initial source: %v", err) + } + ctx := context.Background() + if _, err := svc.Build(ctx, BuildOptions{Dir: root}); err != nil { + t.Fatalf("Build: %v", err) + } + + targetDir := t.TempDir() + target := filepath.Join(targetDir, "outside.go") + if err := os.WriteFile(target, []byte("package sample\n\nfunc Leaked() {}\n"), 0o644); err != nil { + t.Fatalf("write outside target: %v", err) + } + if err := os.Remove(sourcePath); err != nil { + t.Fatalf("remove initial source: %v", err) + } + if err := os.Symlink(target, sourcePath); err != nil { + t.Fatalf("replace source with symlink: %v", err) + } + + syncer := incremental.NewWithRegistry(st, map[string]incremental.Parser{".go": walker}, incremental.WithLogger(slog.Default())) + if _, err := svc.Update(ctx, UpdateOptions{BuildOptions: BuildOptions{Dir: root}, Syncer: syncer}); err != nil { + t.Fatalf("Update: %v", err) + } + assertFunctionNamesByFile(t, st, ctx, "sample.go", []string{"Keep"}) +} + func TestUpdate_MaxFileBytesRejectsLargeFile(t *testing.T) { db, err := gorm.Open(sqlite.Open(":memory:"), &gorm.Config{Logger: gormlogger.Discard}) if err != nil { diff --git a/internal/app/ingest/workflow/packages.go b/internal/app/ingest/workflow/packages.go index f1916dc..332d97c 100644 --- a/internal/app/ingest/workflow/packages.go +++ b/internal/app/ingest/workflow/packages.go @@ -5,7 +5,6 @@ import ( "context" "crypto/sha256" "fmt" - "os" "path" "path/filepath" "slices" @@ -32,7 +31,12 @@ func (s *Service) collectLanguagePackages(ctx context.Context, absDir string, op packages, err := discovery.DiscoverPackages(ctx, ingestapp.PackageDiscoveryOptions{ RootDir: absDir, WalkFiles: func(fn func(path, relPath string) error) error { - return walkMatchingFiles(ctx, absDir, opts, fn) + return walkMatchingFiles(ctx, absDir, opts, func(path, relPath string) error { + if _, err := inspectRegularSourceFile(path); err != nil { + return err + } + return fn(path, relPath) + }) }, HasParser: func(ext string) bool { _, ok := s.parserForExt(ext) @@ -214,7 +218,7 @@ func (s *Service) packageSemanticMetadataForFile(ctx context.Context, absDir, re if !ok { return ingestapp.ParseMetadata{}, "", nil } - content, err := os.ReadFile(filepath.Join(absDir, relPath)) + content, err := readRegularSourceFile(filepath.Join(absDir, relPath)) if err != nil { return ingestapp.ParseMetadata{}, "", err } diff --git a/internal/app/ingest/workflow/spool.go b/internal/app/ingest/workflow/spool.go index bdfbee3..9a84549 100644 --- a/internal/app/ingest/workflow/spool.go +++ b/internal/app/ingest/workflow/spool.go @@ -48,12 +48,13 @@ type spooledUpdateRecord struct { // updateSpool is the temporary on-disk staging area for an incremental update pass. // @intent capture the current file set, hashes, and force-reparse decisions before the update transaction begins. type updateSpool struct { - dir string - records []string - currentFiles map[string]struct{} - currentHashes map[string]string - packages map[string]languagePackageInfo - forceFiles map[string]struct{} + dir string + records []string + currentFiles map[string]struct{} + unreadableFiles map[string]struct{} + currentHashes map[string]string + packages map[string]languagePackageInfo + forceFiles map[string]struct{} } // writeRecord encodes one parsed file as a gob-serialized spool record on disk. diff --git a/internal/app/ingest/workflow/update.go b/internal/app/ingest/workflow/update.go index 8b50651..cf22daf 100644 --- a/internal/app/ingest/workflow/update.go +++ b/internal/app/ingest/workflow/update.go @@ -6,6 +6,7 @@ import ( "crypto/sha256" "encoding/hex" "errors" + "io" "os" "path/filepath" "slices" @@ -141,9 +142,10 @@ func (s *Service) prepareUpdateSpool(ctx context.Context, absDir string, opts Up return nil, trace.Wrap(err, "create update spool") } spool := &updateSpool{ - dir: dir, - currentFiles: make(map[string]struct{}), - currentHashes: make(map[string]string), + dir: dir, + currentFiles: make(map[string]struct{}), + unreadableFiles: make(map[string]struct{}), + currentHashes: make(map[string]string), } batch := make(map[string]ingest.FileInfo) var batchBytes int64 @@ -170,22 +172,30 @@ func (s *Service) prepareUpdateSpool(ctx context.Context, absDir string, opts Up return nil } - info, err := os.Stat(path) + file, info, err := openRegularSourceFile(path) if err != nil { unreadable.add(relPath) + spool.unreadableFiles[relPath] = struct{}{} s.logger().Warn("skip unreadable update file", "file", relPath, "error", err) return nil } if err := CheckParseFileSize(relPath, info.Size(), opts.MaxFileBytes); err != nil { + _ = file.Close() return err } if err := CheckTotalParsedBytes(relPath, totalParsedBytes, info.Size(), opts.MaxTotalParsedBytes); err != nil { + _ = file.Close() return err } - content, err := os.ReadFile(path) + content, err := io.ReadAll(file) + closeErr := file.Close() + if err == nil { + err = closeErr + } if err != nil { unreadable.add(relPath) + spool.unreadableFiles[relPath] = struct{}{} s.logger().Warn("skip unreadable update file", "file", relPath, "error", err) return nil } @@ -260,7 +270,7 @@ func (s *Service) applyUpdateSpoolInTx(ctx context.Context, tx ingest.Transactio useSemiNaiveReplay = ready } } - if requiresFullBuild && s.canBuildForUpdate(opts) && !useSemiNaiveReplay { + if requiresFullBuild && s.canBuildForUpdate(opts) && !useSemiNaiveReplay && len(spool.unreadableFiles) == 0 { return updateOutcome{ stats: classifyUpdateSnapshot(existingFiles, existingNodesByFile, spool.currentHashes), fullBuild: true, @@ -273,12 +283,7 @@ func (s *Service) applyUpdateSpoolInTx(ctx context.Context, tx ingest.Transactio return updateOutcome{}, trace.Wrap(err, "upsert package nodes") } spool.forceFiles = forceFiles - deletedFiles := make([]string, 0, len(existingFiles)) - for _, fp := range existingFiles { - if _, ok := spool.currentFiles[fp]; !ok { - deletedFiles = append(deletedFiles, fp) - } - } + deletedFiles := existingFilesMissingFromSet(spool.currentFiles, spool.unreadableFiles, existingFiles) if stagedSyncer, ok := syncer.(transactionalBatchIncrementalSyncer); ok { stats, err = stagedSyncer.SyncBatchesWithExistingStore(ctx, txStore, newUpdateSpoolBatchSource(ctx, spool), deletedFiles) if err != nil { @@ -451,7 +456,7 @@ func (s *Service) updateGraphWithoutTx(ctx context.Context, absDir string, opts } } - deletedFiles := existingFilesMissingFromSet(spool.currentFiles, existingFiles) + deletedFiles := existingFilesMissingFromSet(spool.currentFiles, spool.unreadableFiles, existingFiles) stats := &ingest.SyncStats{} if stagedSyncer, ok := opts.Syncer.(batchIncrementalSyncer); ok { stats, err = stagedSyncer.SyncBatchesWithExisting(ctx, newUpdateSpoolBatchSource(ctx, spool), deletedFiles) @@ -564,13 +569,18 @@ func affectedUpdateFiles(currentHashes map[string]string, existingNodesByFile ma return files } -// @intent detect deleted paths from the spool snapshot without rebuilding a full in-memory FileInfo map. -func existingFilesMissingFromSet(currentFiles map[string]struct{}, existingFiles []string) []string { +// @intent detect confirmed deleted paths while preserving unreadable files whose current state is unknown. +// @domainRule absence from current files means deletion only when the same path was not observed as unreadable. +func existingFilesMissingFromSet(currentFiles, unreadableFiles map[string]struct{}, existingFiles []string) []string { deleted := make([]string, 0) for _, fp := range existingFiles { - if _, ok := currentFiles[fp]; !ok { - deleted = append(deleted, fp) + if _, current := currentFiles[fp]; current { + continue + } + if _, unreadable := unreadableFiles[fp]; unreadable { + continue } + deleted = append(deleted, fp) } return deleted } diff --git a/internal/app/reposync/admission.go b/internal/app/reposync/admission.go index 49ecd09..afcfd31 100644 --- a/internal/app/reposync/admission.go +++ b/internal/app/reposync/admission.go @@ -2,6 +2,7 @@ package reposync import ( + "fmt" "path" "sort" "strings" @@ -202,6 +203,17 @@ func AllowRulesSpanMultipleOwners(rules []RepoRule) (bool, []string) { return len(owners) > 1 || (len(owners) == 1 && owners[0] == "*"), owners } +// ValidateRepoNameNamespaceRules rejects admission surfaces that can map distinct repositories to one repo-name namespace. +// @intent fail webhook startup before equal repo names from different owners can share checkout and graph state. +// @domainRule repo-name namespaces are safe only when all positive allow rules are constrained to one non-wildcard owner. +func ValidateRepoNameNamespaceRules(rules []RepoRule) error { + spans, owners := AllowRulesSpanMultipleOwners(rules) + if !spans { + return nil + } + return fmt.Errorf("webhook allow-repo owners %v can collide under repo-name namespaces; configure one non-wildcard owner", owners) +} + // @intent apply one compiled allow or deny pattern to a repository full name during filter evaluation. func (r *allowRule) match(repoFullName string) bool { if r.wildcard { diff --git a/internal/app/reposync/admission_test.go b/internal/app/reposync/admission_test.go index dfbdaa6..438d429 100644 --- a/internal/app/reposync/admission_test.go +++ b/internal/app/reposync/admission_test.go @@ -153,6 +153,27 @@ func TestAllowRulesSpanMultipleOwners(t *testing.T) { } } +func TestValidateRepoNameNamespaceRules(t *testing.T) { + tests := []struct { + name string + rules []RepoRule + wantErr bool + }{ + {name: "single owner", rules: []RepoRule{{Pattern: "org/*"}, {Pattern: "!org/private"}}}, + {name: "multiple owners", rules: []RepoRule{{Pattern: "org-a/*"}, {Pattern: "org-b/api"}}, wantErr: true}, + {name: "wildcard owner", rules: []RepoRule{{Pattern: "*/*"}}, wantErr: true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := ValidateRepoNameNamespaceRules(tt.rules) + if (err != nil) != tt.wantErr { + t.Fatalf("ValidateRepoNameNamespaceRules() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +} + func TestRepoFilter_PerRepoBranch_ExactRepo(t *testing.T) { f := NewRepoFilterFromRules([]RepoRule{ {Pattern: "org/api", Branches: []string{"main", "develop"}}, diff --git a/internal/runtime/remote/http.go b/internal/runtime/remote/http.go index ad9b42d..0d53a8c 100644 --- a/internal/runtime/remote/http.go +++ b/internal/runtime/remote/http.go @@ -31,6 +31,13 @@ func RunHTTP(rt *ccgruntime.Runtime, cfg httpin.Config, serviceVersion, ragIndex if cfg.Transport != "streamable-http" { return fmt.Errorf("ccg-server only supports streamable-http transport") } + rules := make([]reposync.RepoRule, 0, len(cfg.AllowRepo)) + for _, raw := range cfg.AllowRepo { + rules = append(rules, reposync.ParseRepoRule(raw)) + } + if err := reposync.ValidateRepoNameNamespaceRules(rules); err != nil { + return err + } inst, err := mcpruntime.New(rt.MCPComponents(), mcpruntime.Options{ CacheTTL: cfg.CacheTTL, NoCache: cfg.NoCache, OTELEndpoint: cfg.OTELEndpoint, NamespaceRoot: cfg.NamespaceRoot, RepoRoot: cfg.RepoRoot, @@ -73,7 +80,7 @@ func RunHTTP(rt *ccgruntime.Runtime, cfg httpin.Config, serviceVersion, ragIndex cleanupQueue := func() {} if len(cfg.AllowRepo) > 0 { - queue, handler, cleanup := buildRepoSyncHTTP(rt, cfg, inst) + queue, handler, cleanup := buildRepoSyncHTTP(rt, cfg, rules, inst) deps.SyncQueue = queue deps.Webhook = handler cleanupQueue = cleanup @@ -85,14 +92,7 @@ func RunHTTP(rt *ccgruntime.Runtime, cfg httpin.Config, serviceVersion, ragIndex // buildRepoSyncHTTP assembles the optional webhook handler and its daemonless worker queue. // @intent centralize remote repository-sync adapter construction and return one idempotent cleanup hook. -func buildRepoSyncHTTP(rt *ccgruntime.Runtime, cfg httpin.Config, inst *mcpruntime.Instance) (*reposync.SyncQueue, http.Handler, func()) { - rules := make([]reposync.RepoRule, 0, len(cfg.AllowRepo)) - for _, raw := range cfg.AllowRepo { - rules = append(rules, reposync.ParseRepoRule(raw)) - } - if spans, owners := reposync.AllowRulesSpanMultipleOwners(rules); spans { - rt.Logger.Warn("webhook allow-repo spans multiple owners; repo-name namespace strategy can collide for equal repo names", "owners", owners, "namespace_strategy", "repo_name") - } +func buildRepoSyncHTTP(rt *ccgruntime.Runtime, cfg httpin.Config, rules []reposync.RepoRule, inst *mcpruntime.Instance) (*reposync.SyncQueue, http.Handler, func()) { filter := reposync.NewRepoFilterFromRules(rules) repoLocker := gitrepo.NewRepoLocker(30 * time.Second) walkers := make(map[string]workflow.Parser, len(rt.Walkers))