Repository navigation
chore(release): v0.12.7 - #1458
Conversation
Found in the v0.12.7 release review. `GET /altimate/trace/:sessionID/view` (#1415) serves the viewer from the API's own origin, and a trace holds tool output and fetched pages. Script injected through that content could have called every other route as same-origin. - The response carries a CSP that allows the page's inline script and styles but no requests (`connect-src 'none'`), form posts or base URL. The embedded viewer is never live, so it makes no requests of its own. Checked in Chromium on a real trace: every tab renders, Copy Summary works, no CSP violations. - The summary tab's rendering error is escaped like the rest of the page. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011z8XLFXDGuywPteBesoZSU
Found in the v0.12.7 release review: #1427's `warehouse-connect` log lines and #1415/#1435's `serve` routes were not documented. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011z8XLFXDGuywPteBesoZSU
…iewer empty Found by this release's adversarial tests, confirmed in Chromium. The viewer embeds the trace as JSON in an inline script and escaped only `</`. Tool output with `<!--<script` and no `-->` after it (a fetched page, say) puts the HTML parser in a state where the script's own `</script>` no longer closes it, so no script ran and the page showed only its frame. Every `<` is now written as `<`, which the parser never reads as markup and JavaScript reads back as the same character. Predates this release (the TUI's `/traces` has the same viewer); the IDE's trace view (#1415) reaches it too. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011z8XLFXDGuywPteBesoZSU
️✅ There are no secrets present in this pull request anymore.If these secrets were true positive and are still valid, we highly recommend you to revoke them. 🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request. |
|
Thanks for your contribution! This PR doesn't have a linked issue. All PRs must reference an existing issue. Please:
See CONTRIBUTING.md for details. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to The adversarial test now expects the viewer’s escaped output while continuing to reject the raw payload. No merge-blocking risk is indicated by this change. Pre-merge checks ·
  |
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 6 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Title check The title clearly identifies this as the v0.12.7 release pull request and matches the primary change. Description check The description follows the required template and provides release scope, change details, verification results, screenshots status, and completed checklist items. Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request.
- Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
-
📝 Generate docstrings 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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.
A rabbit reads the changelog page,
Then checks each trace behind its cage.
The script marks hide from curious sight,
While policies keep requests airtight.
I hop through tests, then rest tonight.
Comment @coderabbitai help to get the list of available commands.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011z8XLFXDGuywPteBesoZSU
9493f3b to
175baa1
Compare
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found and verified against the latest diff
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/opencode/test/server/altimate-trace-routes.test.ts">
<violation number="1" location="packages/opencode/test/server/altimate-trace-routes.test.ts:144">
P2: These substring checks still pass if a permissive duplicate directive appears first; CSP ignores later duplicates, so the test would miss a policy that permits API connections. Assert the complete expected policy or otherwise validate each effective directive.</violation>
</file>
<file name="packages/opencode/test/skill/release-v0.12.7-adversarial.test.ts">
<violation number="1" location="packages/opencode/test/skill/release-v0.12.7-adversarial.test.ts:277">
P2: This test only checks that the inline script contains the `e(...)` call; the supplied trace never reaches the catch, so it cannot catch a regression that renders an error message unsafely. Exercise the summary error path and assert that markup in its message is escaped.</violation>
</file>
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
| expect(csp).toContain("connect-src 'none'") | ||
| expect(csp).toContain("form-action 'none'") | ||
| expect(csp).toContain("default-src 'none'") |
There was a problem hiding this comment.
P2: These substring checks still pass if a permissive duplicate directive appears first; CSP ignores later duplicates, so the test would miss a policy that permits API connections. Assert the complete expected policy or otherwise validate each effective directive.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At packages/opencode/test/server/altimate-trace-routes.test.ts, line 144:
<comment>These substring checks still pass if a permissive duplicate directive appears first; CSP ignores later duplicates, so the test would miss a policy that permits API connections. Assert the complete expected policy or otherwise validate each effective directive.</comment>
<file context>
@@ -135,6 +135,17 @@ describe("GET /altimate/trace/:sessionID/view", () => {
+
+ const response = await get("/altimate/trace/ses_view/view")
+ const csp = response.headers.get("content-security-policy") ?? ""
+ expect(csp).toContain("connect-src 'none'")
+ expect(csp).toContain("form-action 'none'")
+ expect(csp).toContain("default-src 'none'")
</file context>
| expect(csp).toContain("connect-src 'none'") | |
| expect(csp).toContain("form-action 'none'") | |
| expect(csp).toContain("default-src 'none'") | |
| expect(csp).toBe("default-src 'none'; script-src 'unsafe-inline'; style-src 'unsafe-inline'; img-src data: blob:; connect-src 'none'; form-action 'none'; base-uri 'none'") |
|
|
||
| test("the summary tab's rendering error is escaped like the rest of the page", () => { | ||
| const html = renderTraceViewer({ version: 2, traceId: "t", sessionId: "s", metadata: {}, spans: [], summary: {} } as any) | ||
| expect(html).toContain("Summary rendering error: ' + e(err.message || err)") |
There was a problem hiding this comment.
P2: This test only checks that the inline script contains the e(...) call; the supplied trace never reaches the catch, so it cannot catch a regression that renders an error message unsafely. Exercise the summary error path and assert that markup in its message is escaped.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At packages/opencode/test/skill/release-v0.12.7-adversarial.test.ts, line 275:
<comment>This test only checks that the inline script contains the `e(...)` call; the supplied trace never reaches the catch, so it cannot catch a regression that renders an error message unsafely. Exercise the summary error path and assert that markup in its message is escaped.</comment>
<file context>
@@ -0,0 +1,277 @@
+
+ test("the summary tab's rendering error is escaped like the rest of the page", () => {
+ const html = renderTraceViewer({ version: 2, traceId: "t", sessionId: "s", metadata: {}, spans: [], summary: {} } as any)
+ expect(html).toContain("Summary rendering error: ' + e(err.message || err)")
+ })
+})
</file context>
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| const TRACE_PAGE_MAX = 200 | ||
| /** No requests or form posts from the trace viewer page: its content is tool output, not ours. */ | ||
| const TRACE_VIEW_CSP = | ||
| "default-src 'none'; script-src 'unsafe-inline'; style-src 'unsafe-inline'; img-src data: blob:; connect-src 'none'; form-action 'none'; base-uri 'none'" |
There was a problem hiding this comment.
WARNING: The viewer CSP does not prevent navigation with trace data
default-src 'none', connect-src 'none', and form-action 'none' do not restrict top-level navigation. Since script-src 'unsafe-inline' still allows injected handlers, script from an unvalidated trace can redirect to an external URL containing the trace, even though fetch and forms are blocked. For example, Trace.loadTrace parses arbitrary trace JSON, and the viewer interpolates summary.loops[].count into innerHTML without escaping it. Isolate or sandbox this untrusted document rather than treating this policy as complete containment of injected script.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| } | ||
|
|
||
| test("a failure's error is masked and bounded", () => { | ||
| ReconnectLog.resetForTests() |
There was a problem hiding this comment.
WARNING: Direct-handler tests remove the process-wide reconnect listener
resetForTests() unsubscribes the global onReconnect listener and clears all account mappings, although this test only calls ReconnectLog.handle with its own writer. Test files share a process, and Registry.get() returns a cached connector before calling ReconnectLog.install(), so a later reconnect on an existing connector can silently lose its log events after this suite runs. Avoid resetting the global subscription in tests that do not install it, or restore the prior state.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| }) | ||
|
|
||
| test("a '..' path that only names the project is judged where it resolves", () => { | ||
| const escaping = path.join(PROJECT, "..", "evil", ".claude", "skills", "x", "SKILL.md") |
There was a problem hiding this comment.
SUGGESTION: Preserve the traversal in this test input
path.join resolves .. before skillSource sees the location. This actually tests an ordinary /work/evil/... path, not /work/proj/../evil/...; a regression that tests the raw project-path prefix could pass. Construct an unnormalized path containing the .. segment to cover the stated boundary case.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| ### Added | ||
|
|
||
| - **A starter after linking.** `altimate-code link` ends with what the workspace provides (skills, knowledge, integrations, memory setting) and up to three prompts to try; the TUI shows the same after a link and from `/workspace` → **Get started**, where picking a prompt fills the input without sending it. It makes no model call, and anything not yet known is left out rather than shown as "none". `GET /altimate/workspace/starter` serves it to the IDE extension. (#1435) | ||
| - **`learn` can promote lessons without a person, if you opt in.** `learn enable --auto-promote` (or `learn.auto_promote: true`, or `ALTIMATE_LEARN_AUTO_PROMOTE=1`) promotes a lesson an automatic reflection staged when every check passes: it came from a correction, review or CI signal; it is exactly what was staged; it validates like a manual promote; it does not weaken a verification step; it does not edit or remove a lesson a person approved or pinned; and it stays within 3 changes per promotion and 5 per day (`learn.auto_promote_max_changes`, `learn.auto_promote_daily`). Otherwise the candidate stays staged and `learn status` says why. Off by default, needs capture and automatic reflection on, and `learn rollback` undoes a promotion. (#1418) |
There was a problem hiding this comment.
SUGGESTION: Clarify that the daily limit counts promotions
learn.auto_promote_daily checks the number of promotions within 24 hours, not the number of lesson changes. The defaults allow five promotions of up to three changes each, potentially 15 changes per day. Saying 3 changes per promotion and 5 per day suggests a five-change daily limit; specify five automatic promotions per 24 hours instead.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Code Review SummaryStatus: 4 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (9 files)
Fix these issues in Kilo Cloud Previous Review Summary (commit 175baa1)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 175baa1)Status: 4 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (8 files)
Reviewed by gpt-6-sol · Input: 16 · Output: 4.5K · Cached: 409.4K Review guidance: REVIEW.md from base branch |
The viewer now writes every `<` in its embedded trace JSON as `\u003c` (this release's `<!--<script` fix), so the old `<\/script>` form no longer appears. The test's other check, that the raw payload never appears, is unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011z8XLFXDGuywPteBesoZSU
|
🔁 @sahrizvi — this change was promoted to the help-docs hub: https://github.com/AltimateAI/help-docs/pull/122 — please review/merge to keep the docs in sync. |
1 participant
Issue for this PR
Release PR, so there is no issue. It promotes the v0.12.6..
mainpayload tolatestas v0.12.7: #1435, #1418, #1415, #1427, plus the CI-only #1440 and the test-only #1399.Type of change
What does this PR do?
Four commits on top of
main(340df36):release: v0.12.7: the CHANGELOG entry and this release's adversarial tests (test/skill/release-v0.12.7-adversarial.test.ts, 28 tests). The entry opens with a heads-up for support: the start-screen workspace box is back, the skill list changes for every user, the agent may search workspace knowledge on its own, and the newserveroutes need no password unless one is set.fix(server): the trace viewer page may not call the server's API(release review, CTO).GET /altimate/trace/:sessionID/view(feat(observability): serve session traces to the IDE extension #1415) serves the viewer from the API's own origin, and a trace holds tool output and fetched pages. The response now carries a CSP that allows the page's inline script and styles but no requests (connect-src 'none'), form posts or base URL. The embedded viewer is never live, so it makes no requests of its own. The summary tab's rendering error is now escaped too.fix(traces): a trace containing <!--<script no longer renders the viewer empty(found by the adversarial tests). The viewer escaped only</in the trace JSON it embeds. Tool output with<!--<scriptand no-->after it leaves the HTML parser in a state where the script's own</script>no longer closes it, so no script ran. Every<is now<. This predates the release (the TUI's/tracesuses the same viewer).docs:thewarehouse-connectreconnect log lines (fix(warehouse): write Snowflake reconnects to the log #1427) and theserveroutes for the IDE (feat(observability): serve session traces to the IDE extension #1415, feat: workspace skills and knowledge first, a starter after linking, and the start-screen box back #1435).Release review: five reviewers (CTO, product, data engineer, tech lead, and a compliance officer at a customer) all said ship with notes, with no P0. Deferred findings are filed: #1451, #1452, #1453, #1454, #1455, #1456, #1457.
After merge: tag the merge commit
v0.12.7and push only the tag, which startsrelease.yml.How did you verify your code works?
--stage preand--stage tagpassed (9/9 each). Typecheck and marker guard are clean.pty,plugin.loader.shared) under Docker load pass when run alone.HOME.opencode runflake (about 1 run in 4, cf. test: run-process --trace subprocess flakes with "Model not found: test/test-model" while sibling tests pass #1340).ANTHROPIC_API_KEY, 1 is Post-release cleanup: deferred items from v0.9.4 multi-persona review #1052 D14).pre-releasepassed and the darwin-arm64 binary reports0.12.7.<!--<scriptrendered an empty page before the fix and renders fully after it.<escaping or theerr.messageescaping each fails a test.dbt-tools E2Ejob has been red onmainsince 6 Oct (CI: dbt-tools E2E red on main — cached uv env has a dangling interpreter, setup trusts .done marker #1257).release.ymldoesn't depend on it.Screenshots / recordings
N/A
Checklist
🤖 Generated with Claude Code
https://claude.ai/code/session_011z8XLFXDGuywPteBesoZSU
Summary by cubic
Promotes
mainas v0.12.7, shipping workspace skills/knowledge preference, the post-link starter, opt-inlearnauto-promotion, IDE trace routes, and Snowflake reconnect logging, with docs updated to match. Includes two trace-viewer fixes found in the release review: the page could call the server's own API as same-origin, and a trace containing<!--<scriptrendered the viewer empty. After merging, tag the merge commitv0.12.7and push only the tag to startrelease.yml.Bug Fixes
<in the embedded trace JSON, not just</, so tool output like<!--<scriptcan no longer stop the viewer's script from running.Written for commit cdf16dd. Summary will update on new commits.
Summary by CodeRabbit