Skip to content

feat: add per-user knowledge isolation, prompt enhancement, and CI #58

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Koopa0 merged 1 commit into from
Feb 15, 2026
Merged

feat: add per-user knowledge isolation, prompt enhancement, and CI #58

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions .github/dependabot.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
version: 2
updates:
- package-ecosystem: gomod
directory: /
schedule:
interval: weekly
open-pull-requests-limit: 5

- package-ecosystem: github-actions
directory: /
schedule:
interval: weekly
open-pull-requests-limit: 5
167 changes: 58 additions & 109 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,23 +19,8 @@ jobs:
with:
go-version: "1.25"

- name: Run gofmt
run: |
if [ "$(gofmt -l . | wc -l)" -gt 0 ]; then
echo "The following files are not formatted:"
gofmt -l .
exit 1
fi

- name: Run go vet
run: go vet ./...

- name: Install gosec
run: go install github.com/securego/gosec/v2/cmd/gosec@latest

- name: Run gosec
run: gosec ./...

# golangci-lint includes: gofmt, govet, gosec, staticcheck, errcheck, etc.
# No need for standalone gofmt or gosec steps.
- name: Run golangci-lint
uses: golangci/golangci-lint-action@v8
with:
Expand All @@ -45,112 +30,38 @@ jobs:
test:
name: Test
runs-on: ubuntu-latest
strategy:
matrix:
go-version: ["1.25"]

services:
postgres:
image: pgvector/pgvector:pg16
env:
POSTGRES_USER: koopa
POSTGRES_PASSWORD: koopa_dev_password
POSTGRES_DB: koopa_test
options: >-
--health-cmd "pg_isready -U koopa -d koopa_test"
--health-interval 10s
--health-timeout 5s
--health-retries 5
ports:
- 5432:5432

steps:
- name: Checkout code
uses: actions/checkout@v4

- name: Set up Go
uses: actions/setup-go@v5
with:
go-version: ${{ matrix.go-version }}

- name: Cache Go modules
uses: actions/cache@v4
with:
path: ~/go/pkg/mod
key: ${{ runner.os }}-go-${{ hashFiles('**/go.sum') }}
restore-keys: |
${{ runner.os }}-go-

- name: Download dependencies
run: go mod download

- name: Wait for PostgreSQL to be ready
run: |
until pg_isready -h localhost -p 5432 -U koopa; do
echo "Waiting for PostgreSQL..."
sleep 2
done
env:
PGPASSWORD: koopa_dev_password

- name: Run database migrations
run: |
# Create pgvector extension (already in migration but explicit for clarity)
PGPASSWORD=koopa_dev_password psql -h localhost -U koopa -d koopa_test -c "CREATE EXTENSION IF NOT EXISTS vector;"

# Run migrations in order (sorted by filename)
for migration in $(ls db/migrations/*up.sql | sort); do
echo "Running migration: $migration"
PGPASSWORD=koopa_dev_password psql -h localhost -U koopa -d koopa_test -f "$migration"
done

# Verify tables were created
echo "Verifying database schema..."
PGPASSWORD=koopa_dev_password psql -h localhost -U koopa -d koopa_test -c "\dt"
env:
PGPASSWORD: koopa_dev_password
go-version: "1.25"

# Unit tests (excluding integration tests)
- name: Run unit tests
run: |
echo "Running unit tests (excluding integration tests)..."
go test -short -race -coverprofile=coverage-unit.txt -covermode=atomic ./...
run: go test -short -race -coverprofile=coverage.txt -covermode=atomic ./...

# Integration tests (using build tags)
# Only run if GEMINI_API_KEY is available (skip if not set)
- name: Run integration tests
if: env.GEMINI_API_KEY != ''
env:
GEMINI_API_KEY: ${{ secrets.GEMINI_API_KEY }}
DATABASE_URL: postgres://koopa:koopa_dev_password@localhost:5432/koopa_test?sslmode=disable
run: |
echo "Running integration tests with API key..."
go test -tags=integration -race -timeout 15m -coverprofile=coverage-integration.txt -covermode=atomic ./...

# Upload coverage reports
- name: Upload coverage to Codecov
uses: codecov/codecov-action@v4
with:
files: ./coverage-unit.txt,./coverage-integration.txt
files: ./coverage.txt
flags: unittests
name: codecov-umbrella

build:
name: Build
runs-on: ${{ matrix.os }}
strategy:
matrix:
os: [ubuntu-latest, macos-latest, windows-latest]
go-version: ["1.25"]

steps:
- name: Checkout code
uses: actions/checkout@v4

- name: Set up Go
uses: actions/setup-go@v5
with:
go-version: ${{ matrix.go-version }}
go-version: "1.25"

- name: Build (Unix)
if: runner.os != 'Windows'
Expand All @@ -160,17 +71,50 @@ jobs:
if: runner.os == 'Windows'
run: go build -v -o koopa.exe ./

- name: Test binary (Unix)
- name: Smoke test (Unix)
if: runner.os != 'Windows'
run: ./koopa --help

- name: Test binary (Windows)
- name: Smoke test (Windows)
if: runner.os == 'Windows'
run: .\koopa.exe --help

fuzz:
name: Fuzz Tests
govulncheck:
name: Vulnerability Check
runs-on: ubuntu-latest
steps:
- name: Checkout code
uses: actions/checkout@v4

- name: Set up Go
uses: actions/setup-go@v5
with:
go-version: "1.25"

- name: Install govulncheck
run: go install golang.org/x/vuln/cmd/govulncheck@latest

- name: Run govulncheck
run: govulncheck ./...

integration:
name: Integration Tests
runs-on: ubuntu-latest
if: github.event_name == 'pull_request'
services:
postgres:
image: pgvector/pgvector:pg17
env:
POSTGRES_USER: koopa
POSTGRES_PASSWORD: koopa_dev_password
POSTGRES_DB: koopa_test
options: >-
--health-cmd "pg_isready -U koopa -d koopa_test"
--health-interval 10s
--health-timeout 5s
--health-retries 5
ports:
- 5432:5432
steps:
- name: Checkout code
uses: actions/checkout@v4
Expand All @@ -180,16 +124,23 @@ jobs:
with:
go-version: "1.25"

- name: Run fuzz tests (10s each)
- name: Run database migrations
run: |
echo "Running fuzz tests for security-critical inputs..."
go test -fuzz=FuzzPathValidation -fuzztime=10s ./internal/security/ || true
go test -fuzz=FuzzCommandValidation -fuzztime=10s ./internal/security/ || true
go test -fuzz=FuzzIsPathSafe -fuzztime=10s ./internal/security/ || true
PGPASSWORD=koopa_dev_password psql -h localhost -U koopa -d koopa_test -c "CREATE EXTENSION IF NOT EXISTS vector;"
for migration in $(ls db/migrations/*up.sql | sort); do
echo "Running migration: $migration"
PGPASSWORD=koopa_dev_password psql -h localhost -U koopa -d koopa_test -f "$migration"
done

- name: Run integration tests
if: env.GEMINI_API_KEY != ''
env:
GEMINI_API_KEY: ${{ secrets.GEMINI_API_KEY }}
DATABASE_URL: postgres://koopa:koopa_dev_password@localhost:5432/koopa_test?sslmode=disable
run: go test -tags=integration -race -timeout 15m ./...

benchmark:
name: Benchmark Tests
# Only run when PR is labeled with 'benchmark-this'
name: Benchmark
if: contains(github.event.pull_request.labels.*.name, 'benchmark-this')
runs-on: ubuntu-latest
steps:
Expand All @@ -202,9 +153,7 @@ jobs:
go-version: "1.25"

- name: Run benchmarks
run: |
echo "Running benchmarks..."
go test -bench=. -benchmem -benchtime=1s -run=^$ ./... | tee bench.txt
run: go test -bench=. -benchmem -benchtime=1s -run=^$ ./... | tee bench.txt

- name: Upload benchmark results
uses: actions/upload-artifact@v4
Expand Down
31 changes: 31 additions & 0 deletions .github/workflows/fuzz.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
name: Fuzz

on:
schedule:
# Weekly: Sunday 03:00 UTC
- cron: "0 3 * * 0"
workflow_dispatch: # Allow manual trigger

jobs:
fuzz:
name: Fuzz Tests
runs-on: ubuntu-latest
strategy:
fail-fast: false
matrix:
fuzz-target:
- FuzzPathValidation
- FuzzPathValidationWithSymlinks
- FuzzCommandValidation
- FuzzURLValidation
steps:
- name: Checkout code
uses: actions/checkout@v4

- name: Set up Go
uses: actions/setup-go@v5
with:
go-version: "1.25"

- name: Run ${{ matrix.fuzz-target }}
run: go test -fuzz=${{ matrix.fuzz-target }} -fuzztime=30s ./internal/security/
2 changes: 2 additions & 0 deletions db/migrations/000003_add_document_owner.down.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
DROP INDEX IF EXISTS idx_documents_owner;
ALTER TABLE documents DROP COLUMN IF EXISTS owner_id;
6 changes: 6 additions & 0 deletions db/migrations/000003_add_document_owner.up.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
-- Add owner_id to documents for per-user knowledge isolation.
-- Prevents RAG poisoning: user A's stored knowledge cannot influence user B's results.
-- Existing documents get NULL owner_id (legacy/shared — visible to all users).
ALTER TABLE documents ADD COLUMN owner_id TEXT;
Copy link

@codereviewbot-ai codereviewbot-ai bot Feb 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing referential integrity constraints:
The owner_id column is added as TEXT without any constraints (e.g., foreign key to a users table, NOT NULL, or default value). This allows invalid or orphaned values, which may compromise data integrity and security.

Recommendation:
Consider adding a foreign key constraint to ensure owner_id references a valid user, or at minimum document the expected values and add validation at the application layer.

Comment on lines +3 to +4
Copy link

@codereviewbot-ai codereviewbot-ai bot Feb 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ambiguous ownership for legacy data:
The migration leaves existing documents with NULL in owner_id, which may lead to ambiguity in ownership semantics and potential unauthorized access.

Recommendation:
Explicitly define and document the access policy for documents with NULL owner_id, and consider a migration step to assign ownership or mark them as shared/legacy.


CREATE INDEX idx_documents_owner ON documents(owner_id);
5 changes: 5 additions & 0 deletions internal/api/chat.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,11 @@ func (h *chatHandler) streamWithFlow(ctx context.Context, w http.ResponseWriter,
emitter := &jsonToolEmitter{w: w, msgID: msgID}
ctx = tools.ContextWithEmitter(ctx, emitter)
Comment on lines 233 to 234
Copy link

@codereviewbot-ai codereviewbot-ai bot Feb 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potential Data Race on http.ResponseWriter via jsonToolEmitter

The jsonToolEmitter is instantiated and injected into the context, and it wraps the http.ResponseWriter. If the downstream h.flow.Stream implementation or any tool execution emits events from multiple goroutines, this could result in concurrent writes to the same http.ResponseWriter, leading to data races and undefined behavior.

Recommendation:

  • Ensure that all calls to the emitter (and thus to http.ResponseWriter) are performed sequentially from a single goroutine. If concurrent emission is possible, introduce synchronization (e.g., a mutex) within the emitter to serialize writes.


// Inject owner identity for per-user knowledge isolation (RAG poisoning prevention).
if ownerID, ok := userIDFromContext(ctx); ok && ownerID != "" {
ctx = tools.ContextWithOwnerID(ctx, ownerID)
}
Comment on lines +237 to +239
Copy link

@codereviewbot-ai codereviewbot-ai bot Feb 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Security: Owner ID Injection into Context

Injecting the owner ID into the context for per-user knowledge isolation is a good security practice. However, it is crucial to ensure that the owner ID extracted from the context is always validated and cannot be spoofed or manipulated by a malicious client.

Recommendation:

  • Review the implementation of userIDFromContext and the mechanism by which the owner ID is set in the context to ensure it is derived from a trusted authentication source and cannot be overridden by user input.


h.logger.Debug("starting stream", "sessionId", sessionID)

var (
Expand Down
3 changes: 2 additions & 1 deletion internal/chat/chat.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,8 @@ func (a *Agent) generateResponse(ctx context.Context, input string, historyMessa
// Build execute options (using cached toolRefs and languagePrompt)
opts := []ai.PromptExecuteOption{
ai.WithInput(map[string]any{
"language": a.languagePrompt,
"language": a.languagePrompt,
"current_date": time.Now().Format("2006-01-02"),
}),
ai.WithMessagesFn(func(_ context.Context, _ any) ([]*ai.Message, error) {
return messages, nil
Expand Down
Loading
Loading