Skip to content

feat: REST endpoints for vault, tags, sessions, maintenance, analytics (AD-1)#18

Merged
thebtf merged 13 commits into
mainfrom
feat/rest-endpoints-ad1
Mar 19, 2026
Merged

feat: REST endpoints for vault, tags, sessions, maintenance, analytics (AD-1)#18
thebtf merged 13 commits into
mainfrom
feat/rest-endpoints-ad1

Conversation

@thebtf
Copy link
Copy Markdown
Owner

@thebtf thebtf commented Mar 18, 2026

Summary

Implements 14 REST endpoints for features previously only accessible via MCP protocol (AD-1: REST-first architecture). Dashboard will consume these directly.

Depends on: PR #17 (dashboard auth subsystem)

New endpoints (5 handler files)

  • Vault (5): list/get/store/delete credentials + vault status
  • Tags (2): add/remove/set tags + list by tag
  • Sessions (2): list indexed sessions + full-text search transcripts
  • Maintenance (3): trigger consolidation + run maintenance + stats
  • Analytics (1): temporal trends (daily/weekly/hourly)

All handlers delegate to existing services, include swaggo annotations, and follow project patterns.

Test plan

  • GET /api/vault/credentials returns credential list (no values)
  • GET /api/vault/credentials/{name} returns decrypted value
  • POST /api/observations/{id}/tags with action=add updates concepts
  • GET /api/sessions-index returns indexed session list
  • POST /api/maintenance/consolidation triggers consolidation
  • GET /api/analytics/trends?period=daily returns observation counts
  • go build ./cmd/worker/ compiles

Summary by CodeRabbit

Новые функции

  • Система аутентификации на основе API-токенов для клиентов с поддержкой различных уровней доступа
  • Аналитика и визуализация тенденций наблюдений с группировкой по периодам
  • Управление тегами и концепциями для наблюдений
  • Управление защищенным хранилищем учетных данных с шифрованием
  • Поиск и индексация сессий с фильтрацией
  • Эндпоинты для системного обслуживания и оптимизации данных

thebtf added 12 commits March 19, 2026 02:07
Create api_tokens table for client token authentication with bcrypt
hash storage, prefix-based lookup index, and usage tracking columns.
GORM-based store with Create, List, FindByPrefix, Revoke,
IncrementStats, IncrementErrorCount, GetByID, and BatchIncrementStats
methods. APIToken model added to models.go.
Handlers for POST /api/auth/login (master token validation + HMAC cookie),
POST /api/auth/logout (cookie clear), GET /api/auth/me (status check),
GET/POST /api/auth/tokens (list/create), DELETE /api/auth/tokens/:id (revoke).
All handlers include swaggo annotations.
TokenAuth now supports three auth methods: master token (admin),
client API tokens (eng_* prefix with bcrypt verification and scope
enforcement), and HMAC-SHA256 signed session cookies (dashboard).
Read-only tokens are restricted to GET + whitelisted POST endpoints.
HMAC cookie key derived deterministically from master token via SHA-256.
Add public routes (login, logout) and authenticated routes (me, tokens
CRUD) to setupRoutes(). Create TokenStore in initializeAsync() and
wire it into TokenAuth middleware after DB initialization.
Background goroutine reads token IDs from a buffered channel and
flushes accumulated request counts to the database every 5 seconds,
reducing per-request UPDATE overhead for client token authentication.
New handlers in handlers_vault.go:
- GET /api/vault/credentials — list credentials (no values)
- GET /api/vault/credentials/{name} — get with decrypted value
- POST /api/vault/credentials — store credential
- DELETE /api/vault/credentials/{name} — delete credential
- GET /api/vault/status — vault encryption status

All handlers delegate to existing crypto.Vault and ObservationStore.
Includes swaggo annotations for OpenAPI docs.
New handlers in handlers_tags.go:
- POST /api/observations/{id}/tags — add/remove/set tags
- GET /api/observations/by-tag/{tag} — list observations by tag

Logic mirrors MCP tag_observation and get_observations_by_tag handlers.
Includes swaggo annotations for OpenAPI docs.
New handlers in handlers_sessions_rest.go:
- GET /api/sessions-index — list indexed sessions
- GET /api/sessions-index/search — full-text search transcripts

Uses /api/sessions-index path to avoid conflict with existing
/api/sessions live session management routes.
Includes swaggo annotations for OpenAPI docs.
New handlers in handlers_maintenance.go:
- POST /api/maintenance/consolidation — trigger consolidation cycle
- POST /api/maintenance/run — trigger full maintenance
- GET /api/maintenance/stats — maintenance statistics

Delegates to existing consolidationScheduler and maintenanceService.
Includes swaggo annotations for OpenAPI docs.
New handler in handlers_analytics.go:
- GET /api/analytics/trends — temporal trends (obs per period)

Supports daily/weekly/hourly grouping with project filter.
Logic mirrors MCP get_temporal_trends handler.
Includes swaggo annotations for OpenAPI docs.
Wire all new REST handlers into the requireReady + auth route group:
- /api/vault/* (5 endpoints)
- /api/observations/{id}/tags, /api/observations/by-tag/{tag}
- /api/sessions-index, /api/sessions-index/search
- /api/maintenance/* (3 endpoints)
- /api/analytics/trends
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 18, 2026

Important

Review skipped

Review was skipped due to path filters

⛔ Files ignored due to path filters (1)
  • ui/bun.lock is excluded by !**/*.lock

CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including **/dist/** will override the default block on the dist directory, by removing the pattern from both the lists.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: bb1ff276-213a-4203-9108-53400e0aa60a

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Прохождение

Добавляет полнофункциональную систему аутентификации на основе API-токенов с поддержкой токенов, управления сеансами на основе cookies и комплексным набором HTTP-обработчиков для аналитики, хранилища учетных данных, управления тегами, сеансов и обслуживания.

Изменения

Группа / Файл(ы) Резюме
Модель и хранилище токенов БД
internal/db/gorm/migrations.go, internal/db/gorm/models.go, internal/db/gorm/token_store.go
Добавляет миграцию для таблицы api_tokens, GORM-модель APIToken и TokenStore с методами для создания, списания, отзыва и отслеживания статистики использования токенов. Поддерживает поиск по префиксу токена и пакетные обновления счетчиков.
Аутентификация и управление сеансами
internal/worker/middleware.go, internal/worker/handlers_auth.go
Расширяет Middleware для поддержки трехуровневой аутентификации: основного токена, клиентских API-токенов (с проверкой bcrypt и ограничением области действия) и подписанных HMAC-сеансов. Добавляет обработчики для входа, выхода и управления жизненным циклом токенов.
Отслеживание статистики токенов
internal/worker/service.go, internal/worker/token_stats.go
Интегрирует TokenStore в Service, инициирует фоновый планировщик для пакетной передачи метрик использования токенов с интервалом 5 секунд через канал StatsCh.
HTTP-обработчики: аналитика
internal/worker/handlers_analytics.go
Реализует GET /api/analytics/trends для получения наблюдений с группировкой по дням/неделям/часам, обнаружением пиков и вычислением рейтинга топ-концепций. Поддерживает параметры дней (1–365, по умолчанию 30) и валидацию периода.
HTTP-обработчики: хранилище учетных данных
internal/worker/handlers_vault.go
Добавляет пять обработчиков для управления учетными данными (список, получение с расшифровкой, сохранение с шифрованием, удаление, статус). Включает проверку отпечатка ключа шифрования и связь с сеансами хранилища.
HTTP-обработчики: теги
internal/worker/handlers_tags.go
Реализует операции с тегами на наблюдениях (добавление, удаление, замена) и поиск наблюдений по тегу с фильтрацией и ограничением результатов.
HTTP-обработчики: сеансы, обслуживание
internal/worker/handlers_sessions_rest.go, internal/worker/handlers_maintenance.go
Добавляет список/поиск индексированных сеансов и обработчики консолидации, запуска и статистики обслуживания для управления циклом распада и ассоциаций.

Диаграммы последовательности

sequenceDiagram
    participant Client as Клиент
    participant Middleware as Middleware
    participant TokenStore as TokenStore
    participant Database as База данных
    participant Handler as Обработчик

    Client->>Middleware: HTTP запрос с токеном (eng_*)
    Middleware->>TokenStore: FindByPrefix(токен_префикс)
    TokenStore->>Database: Запрос по token_prefix
    Database-->>TokenStore: APIToken (не отозван)
    TokenStore-->>Middleware: APIToken найден

    Middleware->>Middleware: Валидация bcrypt токена
    Note over Middleware: Проверка области действия и revoke статуса
    
    Middleware->>Handler: Передача контекста с область действия
    Handler-->>Client: 200 OK с данными

    Middleware->>TokenStore: IncrementStats (асинхронно через канал)
    Note over TokenStore: StatsCh отправляет ID токена для пакетной обработки
Loading
sequenceDiagram
    participant Client as Клиент
    participant Handler as Обработчик Auth
    participant TokenStore as TokenStore
    participant Database as База данных
    participant StatsFlusher as Планировщик Статистики

    Client->>Handler: POST /api/auth/login (master_token)
    Handler->>Handler: Валидация master_token
    Handler->>Client: 200 OK (подписанный session cookie)

    Client->>Handler: POST /api/auth/tokens/create (имя, область)
    Handler->>TokenStore: Create(имя, хэш, префикс, область)
    TokenStore->>Database: INSERT api_tokens
    Database-->>TokenStore: APIToken с ID
    Handler-->>Client: 200 OK (сырой токен для копирования)

    loop Каждые 5 секунд
        StatsFlusher->>TokenStore: BatchIncrementStats(счета)
        TokenStore->>Database: UPDATE запросы_count, last_used_at
    end
Loading

Оценка усилий при проверке кода

🎯 4 (Сложный) | ⏱️ ~60 минут

Возможно связанные PR

  • security: auth consolidation, vault singleton, cwd hardening, LLM prompt sanitization #10 — обе PR изменяют пути аутентификации и проводки сервиса (internal/worker/middleware.go и internal/worker/service.go), затрагивают одинаковые части кода, связанные с управлением сеансами, токенами и инициализацией хранилища.

Предлагаемые метки

ai-review:passed

Стихотворение

🐰 Крольчонок прыгал через токены в деньках,
С bcrypt защитой и префиксах в руках.
Сеансы подписаны, статистика спешит,
В хранилище данных безопасно все спит! 🔐

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: addition of 14 REST endpoints for vault, tags, sessions, maintenance, and analytics features, with specific reference to the associated issue (AD-1).
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/rest-endpoints-ad1
📝 Coding Plan
  • Generate coding plan for human review comments

Comment @coderabbitai help to get the list of available commands and usage tips.

@gemini-code-assist
Copy link
Copy Markdown

Summary of Changes

Hello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request marks a significant step towards a REST-first architecture by exposing a wide array of core functionalities through new REST endpoints. These include secure management of credentials via the Vault, flexible tagging of observations, efficient listing and searching of indexed sessions, triggering and monitoring maintenance operations, and retrieving temporal analytics. A robust, multi-faceted authentication system has been introduced, supporting both administrative access via master tokens and session cookies, and programmatic access for clients using scoped API tokens. This foundational work enables the dashboard to directly interact with these services, streamlining operations and enhancing overall system accessibility and security.

Highlights

  • New REST Endpoints: Introduced 14 new REST endpoints for Vault, Tags, Sessions, Maintenance, and Analytics functionalities.
  • API Token Authentication: Implemented a comprehensive API token authentication system, including a new api_tokens database table, GORM model, and data store.
  • Enhanced Authentication Middleware: Enhanced the authentication middleware to support master tokens, client API tokens (with read-only/read-write scopes), and HMAC-signed session cookies for dashboard access.
  • Database Migrations: Added database migrations to create the api_tokens table, ensuring proper schema management.
  • Asynchronous Token Stats: Integrated a buffered, asynchronous mechanism for updating API token usage statistics, optimizing database write performance.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a significant number of REST endpoints, exposing vault, tags, sessions, maintenance, and analytics features. The implementation is extensive and covers database migrations, models, stores, and HTTP handlers. The new authentication middleware is robust, supporting master tokens, client API tokens, and session cookies with good security practices like constant-time comparison and secure cookie flags.

My review focuses on correctness, performance, and maintainability. I've identified a few areas for improvement:

  • The analytics endpoint should perform aggregation in the database for better performance and accuracy.
  • A string truncation in the sessions handler is unsafe for multi-byte characters and should be made rune-aware.
  • Some parts of the code could be improved for maintainability by removing magic numbers and using more efficient standard library functions.

Overall, this is a substantial and well-structured feature addition. Addressing these points will enhance the robustness and quality of the new endpoints.

Comment on lines +61 to +144
obs, err := s.observationStore.GetRecentObservations(r.Context(), project, days*50)
if err != nil {
log.Error().Err(err).Msg("get observations for trends failed")
http.Error(w, "get observations: "+err.Error(), http.StatusInternalServerError)
return
}

// Calculate time range
now := time.Now()
startTime := now.AddDate(0, 0, -days)
startEpoch := startTime.UnixMilli()

// Group observations by time bucket
buckets := make(map[string]int)
typeDistribution := make(map[string]int)
conceptCounts := make(map[string]int)
totalInRange := 0

for _, o := range obs {
if o.CreatedAtEpoch < startEpoch {
continue
}
totalInRange++

created := time.UnixMilli(o.CreatedAtEpoch)
var key string
switch groupBy {
case "week":
year, week := created.ISOWeek()
key = fmt.Sprintf("%d-W%02d", year, week)
case "hour_of_day":
key = fmt.Sprintf("%02d:00", created.Hour())
default: // day
key = created.Format("2006-01-02")
}
buckets[key]++

// Track type distribution
typeDistribution[string(o.Type)]++

// Track top concepts
for _, c := range o.Concepts {
conceptCounts[c]++
}
}

// Find peak period
peakPeriod := ""
peakCount := 0
for k, v := range buckets {
if v > peakCount {
peakCount = v
peakPeriod = k
}
}

// Sort and get top concepts
type conceptEntry struct {
name string
count int
}
var topConcepts []conceptEntry
for name, count := range conceptCounts {
topConcepts = append(topConcepts, conceptEntry{name, count})
}
for i := 0; i < len(topConcepts) && i < 10; i++ {
for j := i + 1; j < len(topConcepts); j++ {
if topConcepts[j].count > topConcepts[i].count {
topConcepts[i], topConcepts[j] = topConcepts[j], topConcepts[i]
}
}
}
if len(topConcepts) > 10 {
topConcepts = topConcepts[:10]
}
topConceptsMap := make([]map[string]any, len(topConcepts))
for i, c := range topConcepts {
topConceptsMap[i] = map[string]any{"concept": c.name, "count": c.count}
}

dailyAvg := float64(0)
if days > 0 {
dailyAvg = float64(totalInRange) / float64(days)
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

The current implementation for trend analysis fetches a potentially large number of observations into memory and performs aggregation in Go. This has two main drawbacks:

  1. Inefficiency: Fetching many rows can consume significant memory and CPU, especially for large time ranges. Databases are highly optimized for this type of aggregation.
  2. Inaccuracy: The limit for fetching observations is a rough estimate (days * 50). If the actual number of observations per day is higher, the analysis will be based on incomplete data, leading to incorrect trends.

Consider refactoring this to perform the aggregation directly in the database using GROUP BY queries. This will be more performant, scalable, and guarantee accurate results.

Additionally, the sorting of topConcepts on lines 126-132 uses a bubble sort, which is inefficient (O(n²)). Using sort.Slice would be more idiomatic and performant.

Comment on lines +152 to +156
snippet := res.Session.Content.String
if len(snippet) > 200 {
snippet = snippet[:200]
}
sr.Snippet = snippet
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

Truncating the snippet by slicing bytes (snippet[:200]) is unsafe for UTF-8 encoded strings. It can split a multi-byte character, leading to invalid output and potentially causing issues for the client parsing the JSON response. You should truncate based on runes to ensure character integrity.

			snippet := res.Session.Content.String
			// Truncate by runes to avoid splitting multi-byte characters.
			if len([]rune(snippet)) > 200 {
				snippet = string([]rune(snippet)[:200]) + "..."
			}
			sr.Snippet = snippet

Comment on lines +359 to +370
func containsDuplicateKey(msg string) bool {
for _, s := range []string{"duplicate key", "23505", "UNIQUE constraint"} {
if len(msg) >= len(s) {
for i := 0; i <= len(msg)-len(s); i++ {
if msg[i:i+len(s)] == s {
return true
}
}
}
}
return false
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

This implementation of containsDuplicateKey is unnecessarily complex and inefficient for checking if a string contains a substring. A simple series of strings.Contains calls would be more readable, idiomatic, and performant.

func containsDuplicateKey(msg string) bool {
	// PostgreSQL unique violation error code is 23505.
	// SQLite uses "UNIQUE constraint failed".
	// Check for common phrases.
	return strings.Contains(msg, "duplicate key") || strings.Contains(msg, "23505") || strings.Contains(msg, "UNIQUE constraint")
}

Comment on lines +273 to +277
if len(rawToken) < 12 {
http.Error(w, "unauthorized", http.StatusUnauthorized)
return true
}
prefix := rawToken[4:12]
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The token prefix extraction uses hardcoded magic numbers (12, 4:12). This is brittle and hard to maintain. If the token format constants (tokenRawPrefix, tokenPrefixLen) defined elsewhere in the package change, this logic will break silently. Please use these constants to make the code more robust and readable.

Suggested change
if len(rawToken) < 12 {
http.Error(w, "unauthorized", http.StatusUnauthorized)
return true
}
prefix := rawToken[4:12]
if len(rawToken) < len(tokenRawPrefix)+tokenPrefixLen {
http.Error(w, "unauthorized", http.StatusUnauthorized)
return true
}
prefix := rawToken[len(tokenRawPrefix) : len(tokenRawPrefix)+tokenPrefixLen]

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 13

🧹 Nitpick comments (2)
internal/worker/token_stats.go (1)

35-53: Потенциальный рост памяти при высокой нагрузке.

Если скорость записи в канал превышает частоту flush (5 сек), map pending будет расти без ограничений. При очень высокой нагрузке это может привести к проблемам с памятью.

Рассмотрите добавление счётчика или ограничения на размер pending map с принудительным flush при достижении порога.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@internal/worker/token_stats.go` around lines 35 - 53, The pending map in the
goroutine that reads from ch can grow unbounded under high write rates; add a
size limit and trigger a forced flush when the map reaches a threshold. Modify
the loop around tokenID := <-ch to increment pending[tokenID] and then check
len(pending) against a configurable maxPending (or hardcoded threshold) and call
s.flushTokenStats(pending) and clear the map if exceeded; ensure the same clear
logic is used for the existing ticker.C and ctx.Done branches. Use the existing
symbols (ch, pending, tokenID, ticker.C, ctx.Done, s.flushTokenStats) so the
change is localized to this goroutine.
internal/db/gorm/token_store.go (1)

79-88: Аналогичная проблема в IncrementStats.

Метод не проверяет RowsAffected. Если токен не существует, ошибка не возвращается — UPDATE молча завершается без изменений.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@internal/db/gorm/token_store.go` around lines 79 - 88, IncrementStats
currently performs an UPDATE on the APIToken model but ignores RowsAffected, so
missing tokens are not reported; modify IncrementStats to capture the result
(e.g., res := s.db.WithContext(ctx).Model(&APIToken{}).Where("id = ?",
id).Updates(...)), check res.Error and return it if non-nil, then check
res.RowsAffected and return sql.ErrNoRows (or a domain-specific NotFound error)
when RowsAffected == 0 so callers know the token was not found; keep the update
fields (request_count via gorm.Expr and last_used_at = time.Now()) and use the
same APIToken/IncrementStats symbols to locate the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@internal/db/gorm/models.go`:
- Around line 365-377: The GORM struct tag for APIToken.Scope uses an unquoted
default value (default:read-write) which should be single-quoted to match the
codebase pattern; update the struct tag on the Scope field in the APIToken type
to use default:'read-write' (i.e., change `Scope string     `gorm:"type:text;not
null;default:read-write"` to use single quotes around read-write) so string
defaults are consistently quoted while numeric/bool defaults remain unquoted.

In `@internal/db/gorm/token_store.go`:
- Around line 63-77: In Revoke (method on TokenStore) change the error handling
order: first check and return result.Error if it's non-nil, and only after a
successful query inspect result.RowsAffected and return gorm.ErrRecordNotFound
when RowsAffected == 0; keep the same Updates call on APIToken and preserve the
revoked/revoked_at fields but ensure result.Error is handled before checking
RowsAffected.

In `@internal/worker/handlers_analytics.go`:
- Around line 60-66: The handler currently uses a heuristic limit
GetRecentObservations(r.Context(), project, days*50) which can silently drop
observations inside the time window; replace this with a time-based query that
fetches by timestamp (e.g., call a method like
observationStore.GetObservationsSince(ctx, project, sinceTime) or
observationStore.GetObservationsByTimeRange(ctx, project, start, end)) using
sinceTime := now - days, or implement server-side aggregation to avoid returning
all raw rows; ensure you stop using the days*50 heuristic in the handler and
update any callers to use the new time-filtered method (refer to
observationStore.GetRecentObservations and the handler around this call).

In `@internal/worker/handlers_auth.go`:
- Around line 318-321: The Revoke error handling in the token revoke flow
incorrectly maps every error from tokenStore.Revoke to a 404; change it to
detect the "not found" case (e.g., using errors.Is(err, store.ErrNotFound) or a
tokenStore.IsNotFound(err) helper) and return http.StatusNotFound only for that
specific case, while returning an internal server error (500 or 503 as
appropriate) for other errors, preserving the
log.Error().Err(err).Str("token_id", id).Msg(...) call for all failures; update
the http.Error responses accordingly in the tokenStore.Revoke error branch.

In `@internal/worker/handlers_maintenance.go`:
- Around line 34-44: The current guard uses r.ContentLength > 0, which fails for
chunked requests (ContentLength == -1); instead, remove the ContentLength check
and decode when r.Body != nil, treating io.EOF as an empty body: call
json.NewDecoder(r.Body).Decode(&req) and if the error is io.EOF ignore it (leave
defaults for consolidationRequest/Cycle), otherwise return http.BadRequest on
other decode errors; update handling around consolidationRequest, r.Body, and
req.Cycle accordingly (alternative: you may also validate Content-Type header
before attempting JSON decode).

In `@internal/worker/handlers_sessions_rest.go`:
- Around line 117-122: The limit parsing in the handler uses strconv.Atoi on
r.URL.Query().Get("limit") without an upper bound; modify the logic around
variable limit (and the parsed/err branch) to enforce a maximum cap (e.g.,
maxLimit constant) so parsed values > maxLimit are clamped to maxLimit (and
negative/zero still rejected), ensuring you use the capped value when setting
limit; reference the same parsing flow (r.URL.Query().Get, strconv.Atoi, the
local limit variable) and replace the current assignment with a clamp to [1,
maxLimit].
- Around line 151-157: The current truncation uses byte-slicing (snippet[:200])
on res.Session.Content.String which can cut multibyte UTF-8 characters and
produce invalid UTF-8 in sr.Snippet; change truncation to operate on runes
(convert the string to []rune and take up to 200 runes) or use a UTF-8-aware
helper so that when setting sr.Snippet you truncate by characters/graphemes
rather than bytes, preserving valid UTF-8 and the original checks around
res.Session.Content.Valid and length.
- Around line 32-43: Добавьте верхнюю границу для параметра limit в обработчике
(где используются переменные limit/offset и r.URL.Query().Get("limit")) —
например определить maxLimit (например 100) и при парсинге применять проверку
parsed > 0 && parsed <= maxLimit; если parsed превышает maxLimit, установить
limit = maxLimit (или вернуть ошибку валидации), сохранив поведение по умолчанию
limit = 20; аналогично не трогать offset проверку. Это похоже на поведение в
handleGetObservationsByTag, повторите ту же логику здесь.

In `@internal/worker/handlers_tags.go`:
- Around line 130-174: The Swagger docs list an "offset" query param but
handleGetObservationsByTag never reads or passes it to the search layer; update
handleGetObservationsByTag to parse r.URL.Query().Get("offset"),
validate/convert it to a non-negative int (default 0) and set that value on the
search parameters (e.g., add Offset to search.SearchParams and assign it in the
searchParams struct literal), or alternatively remove the offset param from the
Swagger annotation if pagination by offset is not supported; locate this change
around handleGetObservationsByTag and the search.SearchParams usage.

In `@internal/worker/handlers_vault.go`:
- Around line 40-42: Validate the incoming project string with
ValidateProjectName before passing it into the observationStore calls: check
project via ValidateProjectName(project) in the handlers that call
s.observationStore.ListCredentials (and the other handler spots flagged), and if
validation fails return a 400/validation error response instead of calling the
store. Specifically, in the block using the project variable and in the other
occurrences (the blocks around s.observationStore.ListCredentials and similar
handlers) wrap/replace direct usage with a ValidateProjectName check and
early-return on error to prevent `..` or invalid characters from reaching
observationStore; mirror the existing pattern used in handleGetTrends /
middleware.go.
- Around line 171-184: The validation currently allows a non-empty req.Project
when req.Scope == "global", leading to inconsistent records; update the handler
validation around req.Scope (the block that sets default and switches on
req.Scope) to additionally reject requests where req.Scope == "global" &&
req.Project != "" by returning http.StatusBadRequest with a clear message (e.g.,
"project must be empty for global scope"), and ensure the same guard is added to
the second similar validation block that precedes the StoreObservation call so
StoreObservation(..., req.Project, ...) never receives a project value for
global-scoped entries.
- Around line 132-136: The response returning the plaintext credential via
writeJSON currently lacks cache-control headers; update the handler in
internal/worker/handlers_vault.go (the code that calls writeJSON) to set
response headers preventing caching (e.g., set Cache-Control: no-store and
include Pragma: no-cache and Expires: 0) on the http.ResponseWriter before
invoking writeJSON so browsers and intermediary caches will not store the
secret.

In `@internal/worker/middleware.go`:
- Around line 236-243: The client-token branch (strings.HasPrefix(providedToken,
"eng_") calling ta.authenticateClientToken) lacks admin-route authorization, so
add a route/operation-aware scope check after successful bcrypt/auth in
authenticateClientToken (or immediately after it returns true) that rejects any
token without an explicit admin scope for admin routes (/api/auth/tokens,
/api/maintenance/*, /api/vault/*) and also blocks read-only-scoped tokens from
sensitive GET endpoints that return decrypted credentials; use the token's scope
field (e.g., "read-only", "read-write", or an "admin" flag) to allow or deny,
return the appropriate error response when unauthorized, and centralize this
logic so both the current branch and the other client-token uses (the region
noted 302-321) enforce the same admin-only checks.

---

Nitpick comments:
In `@internal/db/gorm/token_store.go`:
- Around line 79-88: IncrementStats currently performs an UPDATE on the APIToken
model but ignores RowsAffected, so missing tokens are not reported; modify
IncrementStats to capture the result (e.g., res :=
s.db.WithContext(ctx).Model(&APIToken{}).Where("id = ?", id).Updates(...)),
check res.Error and return it if non-nil, then check res.RowsAffected and return
sql.ErrNoRows (or a domain-specific NotFound error) when RowsAffected == 0 so
callers know the token was not found; keep the update fields (request_count via
gorm.Expr and last_used_at = time.Now()) and use the same
APIToken/IncrementStats symbols to locate the change.

In `@internal/worker/token_stats.go`:
- Around line 35-53: The pending map in the goroutine that reads from ch can
grow unbounded under high write rates; add a size limit and trigger a forced
flush when the map reaches a threshold. Modify the loop around tokenID := <-ch
to increment pending[tokenID] and then check len(pending) against a configurable
maxPending (or hardcoded threshold) and call s.flushTokenStats(pending) and
clear the map if exceeded; ensure the same clear logic is used for the existing
ticker.C and ctx.Done branches. Use the existing symbols (ch, pending, tokenID,
ticker.C, ctx.Done, s.flushTokenStats) so the change is localized to this
goroutine.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: a649dd19-0059-48f7-83ac-80b4d32e628c

📥 Commits

Reviewing files that changed from the base of the PR and between a8a3fe9 and de66b70.

📒 Files selected for processing (12)
  • internal/db/gorm/migrations.go
  • internal/db/gorm/models.go
  • internal/db/gorm/token_store.go
  • internal/worker/handlers_analytics.go
  • internal/worker/handlers_auth.go
  • internal/worker/handlers_maintenance.go
  • internal/worker/handlers_sessions_rest.go
  • internal/worker/handlers_tags.go
  • internal/worker/handlers_vault.go
  • internal/worker/middleware.go
  • internal/worker/service.go
  • internal/worker/token_stats.go

Comment on lines +365 to +377
type APIToken struct {
ID string `gorm:"type:uuid;primaryKey;default:gen_random_uuid()"`
Name string `gorm:"type:text;not null;uniqueIndex"`
TokenHash string `gorm:"type:text;not null"`
TokenPrefix string `gorm:"type:text;not null;index"`
Scope string `gorm:"type:text;not null;default:read-write"`
CreatedAt time.Time `gorm:"not null;default:now()"`
LastUsedAt *time.Time `gorm:"column:last_used_at"`
RequestCount int64 `gorm:"not null;default:0"`
ErrorCount int64 `gorm:"not null;default:0"`
Revoked bool `gorm:"not null;default:false"`
RevokedAt *time.Time `gorm:"column:revoked_at"`
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Проверяем, как GORM обрабатывает default тег без кавычек в других моделях
rg -n "default:[a-zA-Z]+-[a-zA-Z]+" --type go

Repository: thebtf/engram

Length of output: 159


🏁 Script executed:

# Посмотрим на все default теги в файле models.go
rg -n "default:" internal/db/gorm/models.go

Repository: thebtf/engram

Length of output: 2670


🏁 Script executed:

# Также проверим наличие других моделей с подобными default значениями
rg -n "default:" --type go | head -20

Repository: thebtf/engram

Length of output: 956


Исправить синтаксис тега default в GORM для строкового значения со специальными символами.

В строке 370 значение default:read-write должно быть заключено в одинарные кавычки: default:'read-write'. Во всем codebase для строковых значений в тегах GORM используются кавычки (например: default:'active', default:'', default:'[]'), в то время как числовые и булевы значения остаются без кавычек. Текущий формат несовместим с этим паттерном и может вызвать проблемы при обработке тега.

Предлагаемое исправление
-	Scope        string     `gorm:"type:text;not null;default:read-write"`
+	Scope        string     `gorm:"type:text;not null;default:'read-write'"`
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@internal/db/gorm/models.go` around lines 365 - 377, The GORM struct tag for
APIToken.Scope uses an unquoted default value (default:read-write) which should
be single-quoted to match the codebase pattern; update the struct tag on the
Scope field in the APIToken type to use default:'read-write' (i.e., change
`Scope string     `gorm:"type:text;not null;default:read-write"` to use single
quotes around read-write) so string defaults are consistently quoted while
numeric/bool defaults remain unquoted.

Comment on lines +63 to +77
// Revoke marks a token as revoked.
func (s *TokenStore) Revoke(ctx context.Context, id string) error {
now := time.Now()
result := s.db.WithContext(ctx).
Model(&APIToken{}).
Where("id = ?", id).
Updates(map[string]interface{}{
"revoked": true,
"revoked_at": &now,
})
if result.RowsAffected == 0 {
return gorm.ErrRecordNotFound
}
return result.Error
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Проверяйте result.Error перед RowsAffected.

В методе Revoke проверка RowsAffected == 0 происходит до проверки result.Error. Если произошла ошибка базы данных, RowsAffected может быть 0 по причине ошибки, а не отсутствия записи.

🐛 Предлагаемое исправление
 func (s *TokenStore) Revoke(ctx context.Context, id string) error {
 	now := time.Now()
 	result := s.db.WithContext(ctx).
 		Model(&APIToken{}).
 		Where("id = ?", id).
 		Updates(map[string]interface{}{
 			"revoked":    true,
 			"revoked_at": &now,
 		})
+	if result.Error != nil {
+		return result.Error
+	}
 	if result.RowsAffected == 0 {
 		return gorm.ErrRecordNotFound
 	}
-	return result.Error
+	return nil
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Revoke marks a token as revoked.
func (s *TokenStore) Revoke(ctx context.Context, id string) error {
now := time.Now()
result := s.db.WithContext(ctx).
Model(&APIToken{}).
Where("id = ?", id).
Updates(map[string]interface{}{
"revoked": true,
"revoked_at": &now,
})
if result.RowsAffected == 0 {
return gorm.ErrRecordNotFound
}
return result.Error
}
// Revoke marks a token as revoked.
func (s *TokenStore) Revoke(ctx context.Context, id string) error {
now := time.Now()
result := s.db.WithContext(ctx).
Model(&APIToken{}).
Where("id = ?", id).
Updates(map[string]interface{}{
"revoked": true,
"revoked_at": &now,
})
if result.Error != nil {
return result.Error
}
if result.RowsAffected == 0 {
return gorm.ErrRecordNotFound
}
return nil
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@internal/db/gorm/token_store.go` around lines 63 - 77, In Revoke (method on
TokenStore) change the error handling order: first check and return result.Error
if it's non-nil, and only after a successful query inspect result.RowsAffected
and return gorm.ErrRecordNotFound when RowsAffected == 0; keep the same Updates
call on APIToken and preserve the revoked/revoked_at fields but ensure
result.Error is handled before checking RowsAffected.

Comment on lines +60 to +66
// Get observations for analysis (rough estimate of limit)
obs, err := s.observationStore.GetRecentObservations(r.Context(), project, days*50)
if err != nil {
log.Error().Err(err).Msg("get observations for trends failed")
http.Error(w, "get observations: "+err.Error(), http.StatusInternalServerError)
return
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Эвристика days*50 занижает метрики.

GetRecentObservations(..., days*50) берёт только приблизительный срез. Если в окне больше 50 * days наблюдений, часть записей внутри диапазона тихо выпадет из расчёта, и distribution, peak_period и top_concepts будут занижены. Для аналитического endpoint лучше запрашивать данные по временному фильтру/агрегации, а не по эвристическому лимиту.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@internal/worker/handlers_analytics.go` around lines 60 - 66, The handler
currently uses a heuristic limit GetRecentObservations(r.Context(), project,
days*50) which can silently drop observations inside the time window; replace
this with a time-based query that fetches by timestamp (e.g., call a method like
observationStore.GetObservationsSince(ctx, project, sinceTime) or
observationStore.GetObservationsByTimeRange(ctx, project, start, end)) using
sinceTime := now - days, or implement server-side aggregation to avoid returning
all raw rows; ensure you stop using the days*50 heuristic in the handler and
update any callers to use the new time-filtered method (refer to
observationStore.GetRecentObservations and the handler around this call).

Comment on lines +318 to +321
if err := tokenStore.Revoke(r.Context(), id); err != nil {
log.Error().Err(err).Str("token_id", id).Msg("auth: failed to revoke token")
http.Error(w, "not found", http.StatusNotFound)
return
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Не маппите любую ошибку Revoke в 404.

Сейчас и «токен не найден», и реальный сбой хранилища уходят клиенту как not found. Это скрывает проблемы БД/сети и ломает семантику API; для внутренних ошибок здесь нужен 500/503.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@internal/worker/handlers_auth.go` around lines 318 - 321, The Revoke error
handling in the token revoke flow incorrectly maps every error from
tokenStore.Revoke to a 404; change it to detect the "not found" case (e.g.,
using errors.Is(err, store.ErrNotFound) or a tokenStore.IsNotFound(err) helper)
and return http.StatusNotFound only for that specific case, while returning an
internal server error (500 or 503 as appropriate) for other errors, preserving
the log.Error().Err(err).Str("token_id", id).Msg(...) call for all failures;
update the http.Error responses accordingly in the tokenStore.Revoke error
branch.

Comment on lines +34 to +44
var req consolidationRequest
// Body is optional; default to "all"
if r.Body != nil && r.ContentLength > 0 {
if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
http.Error(w, "invalid JSON body: "+err.Error(), http.StatusBadRequest)
return
}
}
if req.Cycle == "" {
req.Cycle = "all"
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Ненадёжная проверка ContentLength.

Условие r.ContentLength > 0 не сработает при chunked transfer encoding, где ContentLength равен -1. Это может привести к игнорированию тела запроса.

🐛 Предлагаемое исправление
 	var req consolidationRequest
-	// Body is optional; default to "all"
-	if r.Body != nil && r.ContentLength > 0 {
-		if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
+	// Body is optional; default to "all"
+	if r.Body != nil {
+		if err := json.NewDecoder(r.Body).Decode(&req); err != nil && err.Error() != "EOF" {
 			http.Error(w, "invalid JSON body: "+err.Error(), http.StatusBadRequest)
 			return
 		}
 	}

Альтернативный вариант — проверять Content-Type заголовок.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
var req consolidationRequest
// Body is optional; default to "all"
if r.Body != nil && r.ContentLength > 0 {
if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
http.Error(w, "invalid JSON body: "+err.Error(), http.StatusBadRequest)
return
}
}
if req.Cycle == "" {
req.Cycle = "all"
}
var req consolidationRequest
// Body is optional; default to "all"
if r.Body != nil {
if err := json.NewDecoder(r.Body).Decode(&req); err != nil && err.Error() != "EOF" {
http.Error(w, "invalid JSON body: "+err.Error(), http.StatusBadRequest)
return
}
}
if req.Cycle == "" {
req.Cycle = "all"
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@internal/worker/handlers_maintenance.go` around lines 34 - 44, The current
guard uses r.ContentLength > 0, which fails for chunked requests (ContentLength
== -1); instead, remove the ContentLength check and decode when r.Body != nil,
treating io.EOF as an empty body: call json.NewDecoder(r.Body).Decode(&req) and
if the error is io.EOF ignore it (leave defaults for
consolidationRequest/Cycle), otherwise return http.BadRequest on other decode
errors; update handling around consolidationRequest, r.Body, and req.Cycle
accordingly (alternative: you may also validate Content-Type header before
attempting JSON decode).

Comment on lines +130 to +174
// @Param tag path string true "Tag to search for"
// @Param project query string false "Filter by project"
// @Param limit query int false "Number of results (default 50, max 200)"
// @Param offset query int false "Pagination offset"
// @Success 200 {object} object
// @Failure 400 {string} string "bad request"
// @Failure 500 {string} string "internal error"
// @Router /api/observations/by-tag/{tag} [get]
func (s *Service) handleGetObservationsByTag(w http.ResponseWriter, r *http.Request) {
if s.observationStore == nil {
http.Error(w, "observation store not available", http.StatusServiceUnavailable)
return
}

tag := chi.URLParam(r, "tag")
if tag == "" {
http.Error(w, "tag is required", http.StatusBadRequest)
return
}

project := r.URL.Query().Get("project")
if err := ValidateProjectName(project); err != nil {
http.Error(w, err.Error(), http.StatusBadRequest)
return
}

limit := 50
if val := r.URL.Query().Get("limit"); val != "" {
if parsed, err := strconv.Atoi(val); err == nil && parsed > 0 && parsed <= 200 {
limit = parsed
}
}

if s.searchMgr == nil {
http.Error(w, "search manager not available", http.StatusServiceUnavailable)
return
}

searchParams := search.SearchParams{
Query: tag,
Type: "observations",
Project: project,
Limit: limit,
Concepts: tag,
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Параметр offset объявлен в документации, но не используется.

Параметр offset указан в Swagger-аннотации (строка 133), но не передаётся в SearchParams. Это несоответствие между документацией и реализацией.

🐛 Предлагаемое исправление: удалить неиспользуемый параметр из документации или добавить поддержку
 // `@Param` limit query int false "Number of results (default 50, max 200)"
-// `@Param` offset query int false "Pagination offset"
 // `@Success` 200 {object} object

Или добавить offset в SearchParams, если он поддерживается:

 	searchParams := search.SearchParams{
 		Query:    tag,
 		Type:     "observations",
 		Project:  project,
 		Limit:    limit,
+		Offset:   offset,
 		Concepts: tag,
 	}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// @Param tag path string true "Tag to search for"
// @Param project query string false "Filter by project"
// @Param limit query int false "Number of results (default 50, max 200)"
// @Param offset query int false "Pagination offset"
// @Success 200 {object} object
// @Failure 400 {string} string "bad request"
// @Failure 500 {string} string "internal error"
// @Router /api/observations/by-tag/{tag} [get]
func (s *Service) handleGetObservationsByTag(w http.ResponseWriter, r *http.Request) {
if s.observationStore == nil {
http.Error(w, "observation store not available", http.StatusServiceUnavailable)
return
}
tag := chi.URLParam(r, "tag")
if tag == "" {
http.Error(w, "tag is required", http.StatusBadRequest)
return
}
project := r.URL.Query().Get("project")
if err := ValidateProjectName(project); err != nil {
http.Error(w, err.Error(), http.StatusBadRequest)
return
}
limit := 50
if val := r.URL.Query().Get("limit"); val != "" {
if parsed, err := strconv.Atoi(val); err == nil && parsed > 0 && parsed <= 200 {
limit = parsed
}
}
if s.searchMgr == nil {
http.Error(w, "search manager not available", http.StatusServiceUnavailable)
return
}
searchParams := search.SearchParams{
Query: tag,
Type: "observations",
Project: project,
Limit: limit,
Concepts: tag,
}
// `@Param` tag path string true "Tag to search for"
// `@Param` project query string false "Filter by project"
// `@Param` limit query int false "Number of results (default 50, max 200)"
// `@Success` 200 {object} object
// `@Failure` 400 {string} string "bad request"
// `@Failure` 500 {string} string "internal error"
// `@Router` /api/observations/by-tag/{tag} [get]
func (s *Service) handleGetObservationsByTag(w http.ResponseWriter, r *http.Request) {
if s.observationStore == nil {
http.Error(w, "observation store not available", http.StatusServiceUnavailable)
return
}
tag := chi.URLParam(r, "tag")
if tag == "" {
http.Error(w, "tag is required", http.StatusBadRequest)
return
}
project := r.URL.Query().Get("project")
if err := ValidateProjectName(project); err != nil {
http.Error(w, err.Error(), http.StatusBadRequest)
return
}
limit := 50
if val := r.URL.Query().Get("limit"); val != "" {
if parsed, err := strconv.Atoi(val); err == nil && parsed > 0 && parsed <= 200 {
limit = parsed
}
}
if s.searchMgr == nil {
http.Error(w, "search manager not available", http.StatusServiceUnavailable)
return
}
searchParams := search.SearchParams{
Query: tag,
Type: "observations",
Project: project,
Limit: limit,
Concepts: tag,
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@internal/worker/handlers_tags.go` around lines 130 - 174, The Swagger docs
list an "offset" query param but handleGetObservationsByTag never reads or
passes it to the search layer; update handleGetObservationsByTag to parse
r.URL.Query().Get("offset"), validate/convert it to a non-negative int (default
0) and set that value on the search parameters (e.g., add Offset to
search.SearchParams and assign it in the searchParams struct literal), or
alternatively remove the offset param from the Swagger annotation if pagination
by offset is not supported; locate this change around handleGetObservationsByTag
and the search.SearchParams usage.

Comment on lines +40 to +42
project := r.URL.Query().Get("project")

creds, err := s.observationStore.ListCredentials(r.Context(), project)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Добавьте ValidateProjectName для входного project.

Здесь project из query/body уходит в observationStore без той же проверки, которая уже используется в handleGetTrends и internal/worker/middleware.go. Это оставляет обход общего фильтра для .. и недопустимых символов.

Also applies to: 98-99, 181-184, 253-266

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@internal/worker/handlers_vault.go` around lines 40 - 42, Validate the
incoming project string with ValidateProjectName before passing it into the
observationStore calls: check project via ValidateProjectName(project) in the
handlers that call s.observationStore.ListCredentials (and the other handler
spots flagged), and if validation fails return a 400/validation error response
instead of calling the store. Specifically, in the block using the project
variable and in the other occurrences (the blocks around
s.observationStore.ListCredentials and similar handlers) wrap/replace direct
usage with a ValidateProjectName check and early-return on error to prevent `..`
or invalid characters from reaching observationStore; mirror the existing
pattern used in handleGetTrends / middleware.go.

Comment on lines +132 to +136
writeJSON(w, map[string]any{
"name": name,
"value": plaintext,
"scope": string(cred.Scope),
})
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Отключите кэширование ответа с расшифрованным секретом.

Этот GET возвращает значение credential в открытом виде, но не выставляет Cache-Control: no-store. Браузер или промежуточный кэш могут сохранить секрет вне границ vault.

💡 Возможный фикс
+ w.Header().Set("Cache-Control", "no-store")
  writeJSON(w, map[string]any{
  	"name":  name,
  	"value": plaintext,
  	"scope": string(cred.Scope),
  })
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
writeJSON(w, map[string]any{
"name": name,
"value": plaintext,
"scope": string(cred.Scope),
})
w.Header().Set("Cache-Control", "no-store")
writeJSON(w, map[string]any{
"name": name,
"value": plaintext,
"scope": string(cred.Scope),
})
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@internal/worker/handlers_vault.go` around lines 132 - 136, The response
returning the plaintext credential via writeJSON currently lacks cache-control
headers; update the handler in internal/worker/handlers_vault.go (the code that
calls writeJSON) to set response headers preventing caching (e.g., set
Cache-Control: no-store and include Pragma: no-cache and Expires: 0) on the
http.ResponseWriter before invoking writeJSON so browsers and intermediary
caches will not store the secret.

Comment on lines +171 to +184
if req.Scope == "" {
req.Scope = "project"
}
switch req.Scope {
case "project", "global":
// valid
default:
http.Error(w, "invalid scope: must be \"project\" or \"global\"", http.StatusBadRequest)
return
}
if req.Scope == "project" && req.Project == "" {
http.Error(w, "project is required for project-scoped credentials", http.StatusBadRequest)
return
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

scope="global" должен запрещать project.

Сейчас запрос с scope="global" и непустым project проходит валидацию и затем сохраняется через StoreObservation(..., req.Project, ...). В результате запись получает противоречивое состояние, что легко ломает последующий поиск и удаление.

💡 Возможный фикс
  if req.Scope == "project" && req.Project == "" {
  	http.Error(w, "project is required for project-scoped credentials", http.StatusBadRequest)
  	return
  }
+ if req.Scope == "global" && req.Project != "" {
+ 	http.Error(w, "project must be empty for global-scoped credentials", http.StatusBadRequest)
+ 	return
+ }

Also applies to: 199-212

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@internal/worker/handlers_vault.go` around lines 171 - 184, The validation
currently allows a non-empty req.Project when req.Scope == "global", leading to
inconsistent records; update the handler validation around req.Scope (the block
that sets default and switches on req.Scope) to additionally reject requests
where req.Scope == "global" && req.Project != "" by returning
http.StatusBadRequest with a clear message (e.g., "project must be empty for
global scope"), and ensure the same guard is added to the second similar
validation block that precedes the StoreObservation call so
StoreObservation(..., req.Project, ...) never receives a project value for
global-scoped entries.

Comment on lines +236 to +243
// 1b. Check if it's a client token (eng_* prefix)
if strings.HasPrefix(providedToken, "eng_") && store != nil {
if ta.authenticateClientToken(w, r, next, providedToken, store) {
return
}
// authenticateClientToken wrote the error response if it failed
return
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Добавьте отдельную авторизацию для admin-маршрутов.

После успешной проверки bcrypt единственное ограничение здесь — запрет части изменяющих методов для read-only. Поэтому любой eng_* токен со scope="read-write" проходит к административным маршрутам вроде /api/auth/tokens, /api/maintenance/* и /api/vault/*, а read-only токен — ко всем чувствительным GET, включая выдачу расшифрованных credential'ов. Нужен отдельный admin-only слой авторизации по маршруту/операции или более гранулярные scope'ы.

Also applies to: 302-321

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@internal/worker/middleware.go` around lines 236 - 243, The client-token
branch (strings.HasPrefix(providedToken, "eng_") calling
ta.authenticateClientToken) lacks admin-route authorization, so add a
route/operation-aware scope check after successful bcrypt/auth in
authenticateClientToken (or immediately after it returns true) that rejects any
token without an explicit admin scope for admin routes (/api/auth/tokens,
/api/maintenance/*, /api/vault/*) and also blocks read-only-scoped tokens from
sensitive GET endpoints that return decrypted credentials; use the token's scope
field (e.g., "read-only", "read-write", or an "admin" flag) to allow or deny,
return the appropriate error response when unauthorized, and centralize this
logic so both the current branch and the other client-token uses (the region
noted 302-321) enforce the same admin-only checks.

@thebtf thebtf merged commit 60aab1d into main Mar 19, 2026
2 checks passed
@thebtf thebtf deleted the feat/rest-endpoints-ad1 branch March 19, 2026 05:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant