diff --git a/CHANGELOG.md b/CHANGELOG.md index 9b4cbf5..d172042 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -74,11 +74,31 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 `hybrid`/`rerank`/`recall`/`compress` (hybrid & rerank default on). - Dashboard shows only real, backend-sourced data — removed all mock/hardcoded metrics, dead controls, and the fake auto-setup simulation. +- **Breaking:** `rag_retrieve_context` returns `chunks` as references + (`id`, `score`, `meta`) instead of full chunk objects. The document text was + previously present three times in one response — in `context`, in + `chunks[].Content`, and again in `chunks[].Meta["content"]`. The `context` + string already carries every chunk's text with its score, so callers should + read it from there. +- Search results and exported documents no longer repeat the document text + under `Meta["content"]`; it is returned once, in the content field. +- `rag_list_points` accepts an exact-match `filter` on any metadata key (not + just `source_file`), and `PointInfo` now carries the remaining payload tags + in `meta` — needed when a collection holds agent memory tagged with + `bucket`/`kind`/`ts` rather than a code index. ### Security - `/api/setup/apply` and `/api/setup/test` require localhost or a valid admin token; `/api/setup/apply` no longer echoes the saved config (API key) back. - Removed the deprecated `middleware.RealIP` (X-Forwarded-For spoofing risk). +- `rag_index_project` no longer embeds `.env` files. `.env` was in the + indexable-extension allowlist, so a scanned project's `.env` (and any + `*.env`) was read and sent to the configured embedding provider, then + persisted in the vector store. Sibling forms (`.env.local`, + `.env.production`) were already skipped. A second `isSensitive()` gate now + also blocks names that are otherwise indexable — `secrets.json`, + `db_password.yml`, `credentials.yaml` — plus key material, so the guarantee + no longer depends on the extension allowlist never growing. ### Notes - The Chroma provider is **experimental** (legacy `/api/v1`, mock-tested only); diff --git a/mcp-server/cmd/mcp-server/main.go b/mcp-server/cmd/mcp-server/main.go index e7ab0af..0f98521 100644 --- a/mcp-server/cmd/mcp-server/main.go +++ b/mcp-server/cmd/mcp-server/main.go @@ -195,8 +195,9 @@ type ProjectIDInput struct { } type ListPointsInput struct { - ProjectID string `json:"project_id" jsonschema:"Project identifier"` - SourceFile string `json:"source_file" jsonschema:"Optional: only list chunks from this source file"` + ProjectID string `json:"project_id" jsonschema:"Project identifier"` + SourceFile string `json:"source_file" jsonschema:"Optional: only list chunks from this source file"` + Filter map[string]string `json:"filter" jsonschema:"Optional exact-match metadata filter, e.g. {\"bucket\":\"trackstat\"} or {\"kind\":\"ref\"}. Combined with source_file when both are given."` } type DeletePointsInput struct { @@ -383,7 +384,19 @@ func registerMCPTools(server *mcp.Server, svc *core.Service) { if err != nil { return nil, nil, err } - return nil, map[string]any{"context": context, "chunks": chunks}, nil + // "context" already carries every chunk's text with its score, so the + // chunk list is returned as references only (id, score, metadata). + // Echoing Content here too would send the same text twice in one + // response — the caller pays for it in tokens and gains nothing. + refs := make([]map[string]any, 0, len(chunks)) + for _, c := range chunks { + refs = append(refs, map[string]any{ + "id": c.ID, + "score": c.Score, + "meta": c.Meta, + }) + } + return nil, map[string]any{"context": context, "chunks": refs}, nil }) mcp.AddTool(server, &mcp.Tool{ @@ -432,9 +445,12 @@ func registerMCPTools(server *mcp.Server, svc *core.Service) { mcp.AddTool(server, &mcp.Tool{ Name: "rag_list_points", - Description: "List indexed chunks in a project (id, source file, content preview), optionally filtered by source file. Use to inspect what is stored.", + Description: "List indexed chunks in a project (id, source file, content preview, metadata), optionally filtered by source file and/or an exact-match metadata filter. Use to inspect or enumerate what is stored — including agent memory tagged with bucket/kind/ts.", }, func(ctx context.Context, req *mcp.CallToolRequest, in ListPointsInput) (*mcp.CallToolResult, any, error) { filter := map[string]string{} + for k, v := range in.Filter { + filter[k] = v + } if in.SourceFile != "" { filter["source_file"] = in.SourceFile } diff --git a/mcp-server/pkg/indexer/indexer.go b/mcp-server/pkg/indexer/indexer.go index 0ce5d49..db7f77e 100644 --- a/mcp-server/pkg/indexer/indexer.go +++ b/mcp-server/pkg/indexer/indexer.go @@ -32,20 +32,20 @@ func NewIndexer(provider rag.Provider, chunkSize int) *Indexer { // Default ignore directories. var defaultIgnores = map[string]bool{ - "node_modules": true, - ".git": true, - "vendor": true, - "dist": true, - "build": true, - ".next": true, - ".nuxt": true, - "__pycache__": true, - ".cache": true, - "target": true, - ".idea": true, - ".vscode": true, - ".DS_Store": true, - "coverage": true, + "node_modules": true, + ".git": true, + "vendor": true, + "dist": true, + "build": true, + ".next": true, + ".nuxt": true, + "__pycache__": true, + ".cache": true, + "target": true, + ".idea": true, + ".vscode": true, + ".DS_Store": true, + "coverage": true, ".pytest_cache": true, } @@ -103,6 +103,12 @@ func (idx *Indexer) IndexProject(ctx context.Context, projectID, rootDir string) if !isIndexable(d.Name()) { return nil } + // Never index files that commonly carry secrets, even if their + // extension looks indexable (e.g. a config.json holding API keys). + // Prevents leaking credentials to the external embedding API. + if isSensitive(d.Name()) { + return nil + } relPath, err := filepath.Rel(rootDir, path) if err != nil { @@ -272,7 +278,7 @@ func isIndexable(name string) bool { ".sql": true, ".sh": true, ".bash": true, ".zsh": true, ".yml": true, ".yaml": true, ".toml": true, ".json": true, ".xml": true, ".html": true, ".css": true, ".scss": true, - ".md": true, ".txt": true, ".env": true, ".cfg": true, + ".md": true, ".txt": true, ".cfg": true, ".ini": true, ".conf": true, ".dockerfile": true, ".proto": true, ".graphql": true, ".gql": true, } @@ -286,3 +292,39 @@ func isIndexable(name string) bool { } return false } + +// isSensitive returns true for files that commonly hold secrets and must never +// be embedded or sent to an external embedding API, regardless of extension. +func isSensitive(name string) bool { + n := strings.ToLower(name) + + // Env files: .env, .env.local, .env.production, foo.env, etc. + if n == ".env" || strings.HasPrefix(n, ".env.") || strings.HasSuffix(n, ".env") { + return true + } + + // Well-known credential/secret filenames. + sensitiveNames := map[string]bool{ + ".npmrc": true, ".pypirc": true, ".netrc": true, ".htpasswd": true, + ".pgpass": true, "credentials": true, "id_rsa": true, "id_dsa": true, + "id_ecdsa": true, "id_ed25519": true, + } + if sensitiveNames[n] { + return true + } + + // Substring signals in the filename. + for _, s := range []string{"secret", "credential", "password"} { + if strings.Contains(n, s) { + return true + } + } + + // Key / certificate / keystore extensions. + switch strings.ToLower(filepath.Ext(n)) { + case ".key", ".pem", ".pfx", ".p12", ".ppk", ".keystore", ".jks", ".asc", ".gpg": + return true + } + + return false +} diff --git a/mcp-server/pkg/indexer/indexer_test.go b/mcp-server/pkg/indexer/indexer_test.go index 76f7b3b..03e0809 100644 --- a/mcp-server/pkg/indexer/indexer_test.go +++ b/mcp-server/pkg/indexer/indexer_test.go @@ -6,6 +6,7 @@ import ( "os" "path/filepath" "regexp" + "strings" "sync" "testing" @@ -392,3 +393,81 @@ func TestIndexerListPointsErrorProceedsWithoutSkip(t *testing.T) { t.Error("expected chunks to be indexed even when ListPoints fails") } } + +// TestIsSensitive verifies the predicate that keeps credential-bearing files +// out of the embedding pipeline. +func TestIsSensitive(t *testing.T) { + sensitive := []string{ + ".env", ".env.local", ".env.production", "prod.env", + ".npmrc", ".pypirc", ".netrc", ".htpasswd", ".pgpass", + "credentials", "id_rsa", "id_ed25519", + "my-secret.json", "db_password.yaml", "aws-credentials.txt", + "server.key", "cert.pem", "bundle.pfx", "store.jks", "sig.asc", + ".ENV", "ID_RSA", "My-Secret.JSON", + // Deliberate over-inclusion: the substring match also catches names that + // merely mention a secret. Skipping a doc beats embedding a credential. + "passwordless-auth.md", + } + for _, name := range sensitive { + if !isSensitive(name) { + t.Errorf("isSensitive(%q) = false, want true", name) + } + } + + safe := []string{ + "main.go", "README.md", "config.json", "docker-compose.yml", + "environment.ts", "keyboard.go", + "Makefile", "index.html", + } + for _, name := range safe { + if isSensitive(name) { + t.Errorf("isSensitive(%q) = true, want false", name) + } + } +} + +// TestIndexerSkipsSensitiveFiles is the regression guard that matters: secrets +// on disk must never reach the provider, because Index() ships content to an +// external embedding API. +func TestIndexerSkipsSensitiveFiles(t *testing.T) { + dir := t.TempDir() + + const secret = "RAG_VOYAGE_API_KEY=pa-do-not-embed-me" + files := map[string]string{ + ".env": secret, + ".env.local": secret, + "credentials": secret, + "id_rsa": secret, + "api-secret.json": secret, + "server.key": secret, + "main.go": "package main\n", + } + for name, body := range files { + if err := os.WriteFile(filepath.Join(dir, name), []byte(body), 0644); err != nil { + t.Fatal(err) + } + } + + provider := &mockProvider{} + idx := NewIndexer(provider, 1000) + + result, err := idx.IndexProject(context.Background(), "testproj", dir) + if err != nil { + t.Fatalf("IndexProject: %v", err) + } + + docs := provider.getIndexedDocs() + for _, d := range docs { + if strings.Contains(d.Content, secret) { + t.Fatalf("secret leaked into embedded content: doc %q", d.ID) + } + if sf := d.Meta["source_file"]; isSensitive(sf) { + t.Errorf("sensitive file %q was indexed", sf) + } + } + + // Only main.go should have been scanned. + if result.FilesScanned != 1 { + t.Errorf("FilesScanned = %d, want 1 (only main.go)", result.FilesScanned) + } +} diff --git a/mcp-server/pkg/rag/provider.go b/mcp-server/pkg/rag/provider.go index 882bab3..8ddc113 100644 --- a/mcp-server/pkg/rag/provider.go +++ b/mcp-server/pkg/rag/provider.go @@ -137,4 +137,8 @@ type PointInfo struct { DocID string `json:"doc_id,omitempty"` // original document ID (Qdrant stores UUID, doc_id preserves the original) Content string `json:"content,omitempty"` ChunkIndex string `json:"chunk_index,omitempty"` + // Meta carries any remaining payload fields that are not promoted to a + // field above — e.g. the bucket/kind/title/ts tags used when the store + // holds agent memory rather than a code index. Never includes "content". + Meta map[string]string `json:"meta,omitempty"` } diff --git a/mcp-server/pkg/rag/qdrant.go b/mcp-server/pkg/rag/qdrant.go index c1c6c25..3017510 100644 --- a/mcp-server/pkg/rag/qdrant.go +++ b/mcp-server/pkg/rag/qdrant.go @@ -57,6 +57,11 @@ func NewQdrantProvider(ctx context.Context, baseURL, apiKey string, embedder Emb return p, nil } +// payloadContentKey is the Qdrant payload field holding the document text. +// It is promoted to a dedicated struct field on read, so it must be excluded +// from the generic metadata map to avoid returning the text twice. +const payloadContentKey = "content" + func (p *QdrantProvider) collectionName(projectID string) string { return "project_" + sanitize(projectID) } @@ -188,6 +193,13 @@ func (p *QdrantProvider) SemanticSearch(ctx context.Context, projectID, query st } meta := make(map[string]string, len(r.Payload)) for k, v := range r.Payload { + // "content" is already returned in Result.Content. Copying it into + // Meta as well would ship the full document text twice in every + // search response, roughly doubling the tokens an agent pays to + // read a result. + if k == payloadContentKey { + continue + } if s, ok := v.(string); ok { meta[k] = s } @@ -295,6 +307,21 @@ func (p *QdrantProvider) ListPoints(ctx context.Context, projectID string, metaF if v, ok := pt.Payload["chunk_index"].(string); ok { pi.ChunkIndex = v } + // Surface any remaining payload tags (bucket/kind/title/ts when the + // store holds agent memory). Promoted fields and the document text + // are excluded so nothing is returned twice. + for k, v := range pt.Payload { + switch k { + case payloadContentKey, "source_file", "content_hash", "doc_id", "chunk_index": + continue + } + if s, ok := v.(string); ok { + if pi.Meta == nil { + pi.Meta = make(map[string]string) + } + pi.Meta[k] = s + } + } all = append(all, pi) } if resp.Result.NextOffset == nil { @@ -330,9 +357,15 @@ func (p *QdrantProvider) ExportPoints(ctx context.Context, projectID string) ([] return nil, fmt.Errorf("qdrant export scroll: %w", err) } for _, pt := range resp.Result.Points { - content, _ := pt.Payload["content"].(string) + content, _ := pt.Payload[payloadContentKey].(string) meta := make(map[string]string, len(pt.Payload)) for k, v := range pt.Payload { + // Carried in Document.Content; Index() rewrites the payload's + // content field from there, so keeping a copy in Meta would + // only bloat the migration stream. + if k == payloadContentKey { + continue + } if s, ok := v.(string); ok { meta[k] = s } diff --git a/mcp-server/pkg/rag/qdrant_test.go b/mcp-server/pkg/rag/qdrant_test.go index 241b4cc..3affcd3 100644 --- a/mcp-server/pkg/rag/qdrant_test.go +++ b/mcp-server/pkg/rag/qdrant_test.go @@ -224,3 +224,142 @@ func TestQdrantExportPoints(t *testing.T) { t.Error("metadata not preserved") } } + +// TestQdrantSearchMetaExcludesContent is the token-cost regression guard: the +// document text is promoted to Result.Content, so it must not also appear in +// Result.Meta. Returning both ships the same text twice in every response. +func TestQdrantSearchMetaExcludesContent(t *testing.T) { + const docText = "Title: fleet-notes\n\nThe SCTB counter tracks secret fish." + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/healthz" { + w.WriteHeader(http.StatusOK) + return + } + resp := map[string]any{ + "result": []any{ + map[string]any{ + "id": "abc-123", + "score": 0.87, + "payload": map[string]any{ + "content": docText, + "doc_id": "memory/trackstat/fleet-notes", + "bucket": "trackstat", + "kind": "ref", + "ts": "2026-08-04T10:00+07:00", + }, + }, + }, + } + w.Header().Set("Content-Type", "application/json") + json.NewEncoder(w).Encode(resp) + })) + defer srv.Close() + + p, err := NewQdrantProvider(context.Background(), srv.URL, "", &mockQueryEmbedder{}) + if err != nil { + t.Fatalf("NewQdrantProvider: %v", err) + } + defer p.Close() + + results, err := p.SemanticSearch(context.Background(), "memory", "sctb", 5) + if err != nil { + t.Fatalf("SemanticSearch: %v", err) + } + if len(results) != 1 { + t.Fatalf("expected 1 result, got %d", len(results)) + } + r := results[0] + + if r.Content != docText { + t.Errorf("Content = %q, want the document text", r.Content) + } + if _, present := r.Meta["content"]; present { + t.Error(`Meta["content"] is set: document text returned twice in one response`) + } + for k, v := range r.Meta { + if v == docText { + t.Errorf("Meta[%q] duplicates the document text", k) + } + } + + // The genuinely useful metadata must survive the exclusion. + for k, want := range map[string]string{ + "bucket": "trackstat", + "kind": "ref", + "doc_id": "memory/trackstat/fleet-notes", + } { + if got := r.Meta[k]; got != want { + t.Errorf("Meta[%q] = %q, want %q", k, got, want) + } + } +} + +// TestQdrantListPointsSurfacesMeta verifies that ListPoints exposes the +// bucket/kind/ts tags the agent-memory workflow filters on, while still +// keeping the full document text out of Meta. +func TestQdrantListPointsSurfacesMeta(t *testing.T) { + const docText = "Title: deploy-log\n\nDeployed at 03:00." + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/healthz" { + w.WriteHeader(http.StatusOK) + return + } + resp := map[string]any{ + "result": map[string]any{ + "points": []any{ + map[string]any{ + "id": "pt-1", + "payload": map[string]any{ + "content": docText, + "doc_id": "memory/travelya/deploy-log", + "bucket": "travelya", + "kind": "log", + "ts": "2026-08-04T03:00+07:00", + "source_file": "notes.md", + }, + }, + }, + "next_page_offset": nil, + }, + } + w.Header().Set("Content-Type", "application/json") + json.NewEncoder(w).Encode(resp) + })) + defer srv.Close() + + p, err := NewQdrantProvider(context.Background(), srv.URL, "", &mockQueryEmbedder{}) + if err != nil { + t.Fatalf("NewQdrantProvider: %v", err) + } + defer p.Close() + + points, err := p.ListPoints(context.Background(), "memory", map[string]string{"bucket": "travelya"}) + if err != nil { + t.Fatalf("ListPoints: %v", err) + } + if len(points) != 1 { + t.Fatalf("expected 1 point, got %d", len(points)) + } + pt := points[0] + + if pt.Meta["bucket"] != "travelya" || pt.Meta["kind"] != "log" { + t.Errorf("Meta missing bucket/kind tags: %v", pt.Meta) + } + if pt.Meta["ts"] == "" { + t.Error("Meta[\"ts\"] missing: memory listings sort on it") + } + if _, present := pt.Meta["content"]; present { + t.Error(`Meta["content"] is set: full text leaked into the listing`) + } + // Fields promoted to their own struct field must not be echoed in Meta. + for _, k := range []string{"source_file", "doc_id"} { + if _, present := pt.Meta[k]; present { + t.Errorf("Meta[%q] duplicates a promoted PointInfo field", k) + } + } + if pt.SourceFile != "notes.md" || pt.DocID != "memory/travelya/deploy-log" { + t.Errorf("promoted fields not populated: %+v", pt) + } +}