chore(release): v0.5.0 hardening pass — DEBT-018/020 + security#73
Conversation
--- updated-dependencies: - dependency-name: pytest dependency-version: 9.0.3 dependency-type: direct:development dependency-group: uv - dependency-name: aiohttp dependency-version: 3.13.4 dependency-type: indirect dependency-group: uv - dependency-name: cryptography dependency-version: 46.0.7 dependency-type: indirect dependency-group: uv - dependency-name: lxml dependency-version: 6.1.0 dependency-type: indirect dependency-group: uv - dependency-name: onnx dependency-version: 1.21.0 dependency-type: indirect dependency-group: uv - dependency-name: pillow dependency-version: 12.2.0 dependency-type: indirect dependency-group: uv - dependency-name: pyasn1 dependency-version: 0.6.3 dependency-type: indirect dependency-group: uv - dependency-name: pypdf dependency-version: 6.10.2 dependency-type: indirect dependency-group: uv - dependency-name: python-dotenv dependency-version: 1.2.2 dependency-type: indirect dependency-group: uv - dependency-name: python-multipart dependency-version: 0.0.26 dependency-type: indirect dependency-group: uv - dependency-name: requests dependency-version: 2.33.0 dependency-type: indirect dependency-group: uv ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps the npm_and_yarn group with 1 update in the /vektra-learn/widget directory: [esbuild](https://github.com/evanw/esbuild). Updates `esbuild` from 0.24.2 to 0.25.0 - [Release notes](https://github.com/evanw/esbuild/releases) - [Changelog](https://github.com/evanw/esbuild/blob/main/CHANGELOG-2024.md) - [Commits](evanw/esbuild@v0.24.2...v0.25.0) --- updated-dependencies: - dependency-name: esbuild dependency-version: 0.25.0 dependency-type: direct:development dependency-group: npm_and_yarn ... Signed-off-by: dependabot[bot] <support@github.com>
Three fixes addressing review feedback that was previously deferred to
v0.5.1, now bundled into v0.5.0 to avoid a back-to-back release:
DEBT-018 (widget --vektra-primary scoping):
- Override no longer set on :root (host-page leak); now scoped to
.vektra-chat-btn, .vektra-chat-panel.
- Reuse a single <style id="vektra-primary-override"> node instead of
appending one per construction (multi-instance / hot-reload safety).
DEBT-020 (audit log fallback for sensitive endpoints, NFR-007):
- New helper _resolve_request_id(request) -> UUID in vektra-admin/api.py
and vektra-learn/api.py: returns request.state.request_id or
synthesizes a uuid4(); emits a structlog warning on the fallback path.
- Applied to both writes (POST /api-keys, DELETE /api-keys/{id},
PATCH /admin/namespaces/{id}/config) and reads
(GET /admin/conversations/{id}/turns, GET /learn/.../turns).
- Audit no longer silently skips when the request-id middleware misbehaves.
- Unit tests cover the helper and the fallback path on a sensitive read.
Test fakes (4 LOW from code-quality bot):
- Renamed self_inner -> self in nested _FakeResult / _FakeSession used
by test_fetch_document_names_marks_archived. Pure style; no behaviour
change.
uv.lock:
- Regenerated to match the v0.5.0 pyproject.toml after cherry-picking
the dependency bump from PR #68 (litellm, pytest, etc.).
Updates [0.5.0] entry to reflect the additional fixes bundled into the release instead of deferring to v0.5.1: - Date moved to 2026-04-27 (actual ship date) - Changed: widget --vektra-primary scope to widget roots (DEBT-018) - Fixed: audit log fallback on sensitive admin/learn endpoints (DEBT-020), test fakes self_inner -> self - Security: litellm critical CVE fix, pytest 9, esbuild 0.25
Found during self-review of the v0.5.0-finalize branch: vektra-ingest had two private helpers (_write_audit_log_async, _write_audit_log_direct) with the same `if request_id is None: return` pattern that DEBT-020 was meant to fix everywhere. They were silently skipping audit on every ingest endpoint when the request-id middleware misbehaves. Same treatment as vektra-admin/api.py and vektra-learn/api.py: - Add private _resolve_request_id helper (uuid4 fallback + structlog warning). - Use it in both writers, dropping the early-return. Updated DEBT-020 BACKLOG entry to status=completed and broaden the resolution narrative. Updated CHANGELOG to list the ingest endpoints (POST /api/v1/ingest, batch ingest, deletion) alongside the admin/learn endpoints already covered. Marked DEBT-018 completed too. No new tests for vektra-ingest: the helper is byte-identical to the admin one, which already has unit coverage.
Found during second-pass review: test_resolve_request_id_synthesizes_ uuid_when_missing took a caplog parameter but never asserted on it. Verifying structlog warnings emission is best done with structlog's own testing utilities, not stdlib caplog. Minor cleanup; the test still exercises the synthesis path and asserts UUID type + uniqueness.
fvadicamo
added this to the
v0.5.0 - Widget production-ready & instructor configuration milestone
fvadicamo
added
documentation
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR introduces consistent request-ID fallback handling across admin, ingest, and learn modules by adding Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements technical debt resolutions and security updates for the v0.5.0 release. Key improvements include scoping the widget's primary color CSS variable to prevent host-page style leakage and introducing a robust audit logging fallback mechanism across the admin, learn, and ingest modules to ensure sensitive actions are always recorded. Dependencies such as litellm and pytest were bumped to address security vulnerabilities and modernize the test environment. Feedback was provided regarding the widget's style injection logic, noting that the current use of a global style ID may cause conflicts in multi-instance scenarios where different primary colors are required.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (4)
vektra-learn/src/vektra_learn/api.py (1)
60-76: Consider hoisting_resolve_request_idintovektra_shared.This helper is duplicated almost verbatim across
vektra-learn/src/vektra_learn/api.py,vektra-ingest/src/vektra_ingest/api.py, andvektra-admin/src/vektra_admin/api.py. Since the import-linter contracts allow each component to import fromvektra_shared, a singlevektra_shared.audit.resolve_request_id(request) -> UUID(or similar) would remove the triplication and ensure the three audit-fallback paths can never drift in behavior or log key. Acceptable to defer, but worth a follow-up.Also note this copy calls
structlog.get_logger(__name__).warning(...)on every fallback invocation while the other two copies use a module-levellog— minor consistency nit if you keep the per-module copies.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@vektra-learn/src/vektra_learn/api.py` around lines 60 - 76, The helper function _resolve_request_id in vektra_learn.api is duplicated across services; extract it into a shared helper (e.g., vektra_shared.audit.resolve_request_id(request) -> UUID) and replace the local _resolve_request_id with an import call to that shared function so all services share one implementation and audit-fallback behavior; when moving it, preserve the current logic (return request.state.request_id or uuid4(), emit a warning on fallback) and standardize the logger usage (use a module-level log variable or identical logging call/signature as the other services) to keep log keys consistent.vektra-admin/src/vektra_admin/api.py (1)
374-385: Minor:is_bootstrap_key(token)is recomputed.
_bootstrap.is_bootstrap_key(token)is already evaluated at line 279; re-calling it at line 375 is harmless but easy to avoid by caching the result once near the top of the function. Optional cleanup.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@vektra-admin/src/vektra_admin/api.py` around lines 374 - 385, The code calls _bootstrap.is_bootstrap_key(token) twice; compute it once near the start of the function and reuse that boolean variable (e.g., is_bootstrap) when scheduling the background task instead of calling _bootstrap.is_bootstrap_key(token) again; update the invocation passed to _audit.log_event (in the background_tasks.add_task call) to use the cached is_bootstrap value.vektra-learn/widget/src/chat-ui.js (1)
136-154: Override is deduped, but the main<style>block still accumulates.The DEBT-018 fix correctly scopes
--vektra-primaryto.vektra-chat-btn/.vektra-chat-paneland reuses a single#vektra-primary-overridenode. However, the comment ("repeated construction (hot reload, multi-instance, host-page re-init) doesn't accumulate style elements") reads as if it applies to both blocks, while in practice the main style at lines 137–139 still appends a fresh<style>on everyChatUIconstruction. If hot-reload/multi-instance accumulation is a real concern, the same dedup pattern should be applied to the main block too; otherwise consider tightening the comment to scope it to the override only.♻️ Possible follow-up to dedupe the main style node as well
_injectStyles() { - const style = document.createElement("style"); - style.textContent = buildStyles(this._theme); - document.head.appendChild(style); + let style = document.getElementById("vektra-base-styles"); + if (!style) { + style = document.createElement("style"); + style.id = "vektra-base-styles"; + document.head.appendChild(style); + } + style.textContent = buildStyles(this._theme);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@vektra-learn/widget/src/chat-ui.js` around lines 136 - 154, The main style block in _injectStyles() always appends a new <style>, causing accumulation; change it to reuse/dedupe a single style node (e.g., look up document.getElementById("vektra-chat-ui-styles") and if missing create/appended it, otherwise update its textContent = buildStyles(this._theme)) instead of always creating a fresh element, referencing the _injectStyles method, buildStyles(this._theme) and this._theme; alternatively, if you prefer not to dedupe, update the existing comment to explicitly state it only refers to the primary-override node.vektra-core/pyproject.toml (1)
11-11: Relax the overly restrictivelitellmversion pin.
litellm>=1.83.10,<1.83.11locks to exactly 1.83.10, but the OIDC userinfo cache key collision CVE (CVE-2026-35030) was patched in v1.83.0, not v1.83.10. Versions 1.83.10 and later all include the fix. Consider relaxing the constraint to>=1.83.0(or>=1.83.10if other constraints apply) without the<1.83.11upper bound, allowing patch releases to flow downstream without requiring a new PR.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@vektra-core/pyproject.toml` at line 11, The pinned dependency "litellm>=1.83.10,<1.83.11" is too restrictive; update the pyproject.toml dependency line for litellm (the existing string literal) to relax the constraint to a broader range such as "litellm>=1.83.0" (or at minimum remove the upper bound to "litellm>=1.83.10") so downstream patch releases can flow without PRs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@vektra-admin/src/vektra_admin/api.py`:
- Around line 374-385: The code calls _bootstrap.is_bootstrap_key(token) twice;
compute it once near the start of the function and reuse that boolean variable
(e.g., is_bootstrap) when scheduling the background task instead of calling
_bootstrap.is_bootstrap_key(token) again; update the invocation passed to
_audit.log_event (in the background_tasks.add_task call) to use the cached
is_bootstrap value.
In `@vektra-core/pyproject.toml`:
- Line 11: The pinned dependency "litellm>=1.83.10,<1.83.11" is too restrictive;
update the pyproject.toml dependency line for litellm (the existing string
literal) to relax the constraint to a broader range such as "litellm>=1.83.0"
(or at minimum remove the upper bound to "litellm>=1.83.10") so downstream patch
releases can flow without PRs.
In `@vektra-learn/src/vektra_learn/api.py`:
- Around line 60-76: The helper function _resolve_request_id in vektra_learn.api
is duplicated across services; extract it into a shared helper (e.g.,
vektra_shared.audit.resolve_request_id(request) -> UUID) and replace the local
_resolve_request_id with an import call to that shared function so all services
share one implementation and audit-fallback behavior; when moving it, preserve
the current logic (return request.state.request_id or uuid4(), emit a warning on
fallback) and standardize the logger usage (use a module-level log variable or
identical logging call/signature as the other services) to keep log keys
consistent.
In `@vektra-learn/widget/src/chat-ui.js`:
- Around line 136-154: The main style block in _injectStyles() always appends a
new <style>, causing accumulation; change it to reuse/dedupe a single style node
(e.g., look up document.getElementById("vektra-chat-ui-styles") and if missing
create/appended it, otherwise update its textContent = buildStyles(this._theme))
instead of always creating a fresh element, referencing the _injectStyles
method, buildStyles(this._theme) and this._theme; alternatively, if you prefer
not to dedupe, update the existing comment to explicitly state it only refers to
the primary-override node.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f6dbc953-79b8-4125-b6a3-7498773e69ee
⛔ Files ignored due to path filters (3)
.s2s/BACKLOG.mdis excluded by!.s2s/**uv.lockis excluded by!**/*.lock,!**/*.lockvektra-learn/widget/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (10)
CHANGELOG.mdpyproject.tomlvektra-admin/src/vektra_admin/api.pyvektra-admin/tests/test_admin_turns.pyvektra-core/pyproject.tomlvektra-core/tests/test_pipeline.pyvektra-ingest/src/vektra_ingest/api.pyvektra-learn/src/vektra_learn/api.pyvektra-learn/widget/package.jsonvektra-learn/widget/src/chat-ui.js
Three trivial fixes addressing CodeRabbit nitpicks. Deferred items (Gemini medium #1 multi-instance widget, CodeRabbit nitpick #2 helper hoist) tracked as DEBT-022 / DEBT-023 in BACKLOG. Fix #3: dedupe `is_bootstrap_key(token)` call in vektra-admin/api.py. The check now runs once near the top of `create_api_key` and the cached `is_bootstrap` boolean is reused in the audit metadata. Fix #4: tighten the `_injectStyles` comment in chat-ui.js. The previous wording read as if both the main style block and the primary-color override were deduped; only the override is. Comment now states this explicitly and points to DEBT-022 for the multi-instance follow-up. Fix #5: relax `litellm` pin in vektra-core/pyproject.toml from `>=1.83.10,<1.83.11` to `>=1.83.10`. The exact-version pin came from Dependabot's auto-bump; removing the upper bound lets future patch releases flow without a new PR. Lockfile regenerated; resolved version unchanged (1.83.10). BACKLOG: added DEBT-022 (multi-instance widget primary-color + main-style dedupe) and DEBT-023 (hoist _resolve_request_id to vektra_shared) to track the deferred review items.
|
Addressed CodeRabbit's 4 nitpicks in
Plus deferred from the inline thread:
|
fvadicamo
left a comment
fvadicamo
left a comment
There was a problem hiding this comment.
Review feedback addressed in ff2a745:
- 1 Gemini medium (multi-instance widget): deferred → DEBT-022
- 4 CodeRabbit nitpicks: 3 fixed inline (is_bootstrap_key dedup, comment scope, litellm pin relax), 1 deferred → DEBT-023 (hoist helper to vektra_shared)
Both deferred items now tracked in BACKLOG with full context, acceptance criteria, and proposed approach.
Self-review caught a third recompute of `_bootstrap.is_bootstrap_key(token)` at line 351 of `create_api_key`, used to gate the bootstrap-key consumption. CodeRabbit only flagged the line 279 + line 375 pair (now using the cache), but the function had a third independent call to the same function for the same token, which can also reuse the cache. Now `_bootstrap.is_bootstrap_key(token)` is called exactly once per request (at the top of the function), and three downstream branches (auth flow, audit metadata, bootstrap consumption) reuse the cached boolean.
1 participant
Summary
Pre-release hardening pass on v0.5.0 that bundles into the release everything we'd otherwise have shipped as v0.5.1 within 24h, avoiding the back-to-back release ceremony.
litellm(auth bypass via OIDC userinfo cache key collision) closed via dependency bump--vektra-primaryno longer leaks to host page:root)self_inner->self)Why
When PR #71 (release develop -> main) was reviewed, Gemini and code-quality bots posted feedback that we initially deferred to v0.5.1 per a "zero new commits on release PR" policy. Reading the actual content, almost all of it had moderate-to-high value (litellm critical CVEs, NFR-007 audit gaps, widget host-page leak), and shipping v0.5.0 followed by v0.5.1 within a day projects "release-then-fix" rather than "release-and-ready".
Per discussion this PR closes the gap: ship a single, tighter v0.5.0 with the deferred fixes inside, drop the parallel v0.5.1 plan. Same approach is being mirrored on vektra-moodle for consistency.
What's included
Security (deps)
litellm>=1.40,<1.82.7->>=1.83.10,<1.83.11— closes 2 critical CVEs (auth bypass via OIDC userinfo cache key collision) and several high-severity findings (authenticated command execution via MCP stdio test endpoints, SSTI in/prompts/test).pytest>=8.0->>=9.0.3(full suite green on the new major version).esbuild^0.24->^0.25.Cherry-picks of #68 (uv group, 11 updates) and #40 (esbuild widget). Other open Dependabot PRs (#39, #61) intentionally deferred.
Application fixes
--vektra-primaryscoping: the override is now applied to widget roots (.vektra-chat-btn, .vektra-chat-panel) instead of:root, so it no longer clobbers any unrelated--vektra-primarydeclarations on the host page. Repeated initialization reuses a single<style id="vektra-primary-override">element instead of accumulating duplicates.vektra-admin/api.py,vektra-learn/api.py, andvektra-ingest/api.pynow declares a private_resolve_request_id(request) -> UUIDhelper that synthesizesuuid4()whenrequest.state.request_idis missing and emits arequest_id_middleware_missing_fallbackstructlog warning. Applied to all sensitive endpoints discovered during sweep:POST /api-keys,DELETE /api-keys/{id},PATCH /admin/namespaces/{id}/config,GET /admin/conversations/{id}/turnsGET /api/v1/learn/conversations/{id}/turns_write_audit_logand_write_audit_log_directwriters used byPOST /api/v1/ingest, batch ingest, deletionStyle
vektra-core/tests/test_pipeline.py: renamedself_inner->selfin nested_FakeResult/_FakeSessiontest helpers (4 LOW nitpicks from code-quality bot on PR release: v0.5.0 - Widget production-ready & instructor configuration #71).Tests
vektra-admin/tests/test_admin_turns.py. The existingvektra-learn/tests/test_api.pyalready validates UUID synthesis on the learn path (preserved).Docs
[0.5.0]updated with### Fixedand### Securitysections; date moved to 2026-04-27 (actual ship date).completedwith resolution notes; DEBT-021 (_fetch_document_namesJOIN) remainsplanned(deferred — low priority, no measurable latency impact).Test plan
make lintgreen (8/8 import-linter contracts kept)make test— 632 passed locally (4 pre-existingvektra-shared/test_config.pyfailures from local.envleakage, verified green on CI in clean env)Refs
Summary by CodeRabbit
Bug Fixes
New Features
Chores
pytest(9.0.3),litellm(1.83.10), andesbuild(0.25).