fix(security): harden dashboard trust boundaries - #364
Conversation
mira-2026
added
priority: high
|
Warning Review limit reached
Next review available in: 36 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR hardens Dashboard runtime behavior across release execution, Gateway requests, filesystem and route validation, service resilience, canonical chat data, frontend state, and regression coverage. It also pins GitHub Actions to immutable commits. ChangesDashboard hardening and runtime updates
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
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 |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
backend/test/chatCanonicalMessage.test.ts (1)
15-37: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winValidate provider attachment URLs before using them.
AttachmentPreviewContentpassespreviewItem.urlintochatImageDisplayUrl(), and that value is used as the<img src>source. The downloaded file name is taken from provider data. Reject URL values that are not data URLs or trusted dashboard origins before rendering or downloading, or explicitly document that this contract is only safe for trusted provider transcripts.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/test/chatCanonicalMessage.test.ts` around lines 15 - 37, Validate provider-supplied attachment URLs in the flow used by AttachmentPreviewContent and chatImageDisplayUrl before rendering them as img src or using them for downloads. Accept only data URLs or URLs from trusted dashboard origins, rejecting arbitrary absolute or untrusted URLs; ensure the existing canonicalChatImageDisplayUrl and related normalization paths enforce this contract consistently.frontend/src/components/features/chat/chatTypes.ts (1)
297-461: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winExclude SVG data URLs from the embedded raster dimension check.
embeddedChatImageDimensionsonly checks PNG, GIF, JPEG, and WebP bytes, sodata:image/svg+xml;base64,...URLs failisEmbeddedChatImageWithinDimensionLimit.chatImageDisplayUrlthen returnsundefinedbefore reaching the SVG MIME branch, so embedded SVG images fail to render.Exempt
data:image/svg+xmlfrom the raster gate, with SVG-specific size validation if needed.Add a regression test for an embedded base64 SVG data URL passing through
chatImageDisplayUrl.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/components/features/chat/chatTypes.ts` around lines 297 - 461, Update chatImageDisplayUrl so data:image/svg+xml URLs bypass the raster-only isEmbeddedChatImageWithinDimensionLimit check, while PNG, GIF, JPEG, and WebP continue using it; retain any existing SVG MIME handling and apply SVG-specific size validation if required. Add a regression test covering an embedded base64 SVG data URL that successfully passes through chatImageDisplayUrl.
🧹 Nitpick comments (6)
backend/src/services/databaseOverview.ts (1)
424-442: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winLog the rejected torrent-count queries.
A rejected query now reports
0. Consumers cannot distinguish an empty table from a failed probe, and the rejection reason is discarded. Add a log line for the rejected result so operators can diagnose the failure.♻️ Proposed refactor
- const countFromResult = (result: PromiseSettledResult<string>) => - result.status === "fulfilled" - ? numberFrom( - stringFallback( - parseTable<{ count: string }>(result.value)[0]?.count, - "0" - ) - ) - : 0; + const countFromResult = (label: string, result: PromiseSettledResult<string>) => { + if (result.status === "rejected") { + console.warn(`Torrent count query failed for ${label}:`, result.reason); + return 0; + } + return numberFrom( + stringFallback(parseTable<{ count: string }>(result.value)[0]?.count, "0") + ); + }; return { - comet: countFromResult(cometResult), - bitmagnet: countFromResult(bitmagnetResult), + comet: countFromResult("comet", cometResult), + bitmagnet: countFromResult("bitmagnet", bitmagnetResult), };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/services/databaseOverview.ts` around lines 424 - 442, Update the torrent-count result handling around countFromResult so rejected Promise.allSettled results are logged with their rejection reason before returning 0. Preserve the existing fulfilled-result parsing and zero fallback, and use the surrounding database overview logging mechanism.frontend/src/pages/Delivery.tsx (1)
1468-1472: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winInclude the pull-request refetch in the refresh button state.
isLoadingtracks onlyreconcilePullRequestPreview.isPending. The spinner stops and the button re-enables whilerefetchPullRequestsis still running, so a second refresh can start. ExposeisFetchingfromusePullRequestsand combine both states. Consider disabling the control whileisActionPendingis true, which matches the other controls in this header.♻️ Proposed refactor
<RefreshButton onClick={() => void refreshDelivery()} - isLoading={reconcilePullRequestPreview.isPending} + isLoading={ + reconcilePullRequestPreview.isPending || + isPullRequestsFetching + } label="Refresh Delivery" />Destructure the fetch state where the query is read:
const { data: pullRequests = [], isLoading, + isFetching: isPullRequestsFetching, error, refetch: refetchPullRequests, } = usePullRequests();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/pages/Delivery.tsx` around lines 1468 - 1472, Update the Delivery page refresh flow around RefreshButton and usePullRequests so its loading state remains active while either reconcilePullRequestPreview is pending or pull-request refetching is in progress. Destructure and reuse the hook’s isFetching state, and include isActionPending in the disabled/loading guard consistently with the other header controls.backend/test/openClawChatBridge.test.ts (1)
385-385: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the retention cap, not just "fewer than the fixture".
toBeLessThan(itemEventCount)passes for any retained count between 0 and 249. A regression that raises or removes the eviction cap still passes the test.Assert the configured retention limit directly, ideally by importing the constant that the bridge uses.
♻️ Proposed tighter assertion
- expect(itemToolCount).toBeLessThan(itemEventCount); + expect(itemToolCount).toBe(MAX_RETAINED_ITEM_TOOL_EVENTS);Export the retention limit from the bridge module and import it here.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/test/openClawChatBridge.test.ts` at line 385, Update the retention assertion in openClawChatBridge.test.ts to compare itemToolCount directly against the bridge’s configured retention-limit constant, rather than only itemEventCount. Export that retention limit from the bridge module if necessary, import it in the test, and assert the retained count does not exceed it.contracts/chatCanonicalUtilities.ts (1)
143-168: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winPreserve distinct object keys after key truncation.
Two keys longer than 4096 characters truncate to the same bounded key. The later value then overwrites the earlier value without any marker. A provider key named
[truncated]is also overwritten by the truncation marker at Line 146. Diagnostics then show fewer properties than the provider sent.Disambiguate a bounded key when it already exists.
♻️ Proposed fix to keep truncated keys distinct
const boundedKey = truncateCanonicalChatText( key, Math.min(4096, budget.remainingStringCharacters) ); + let uniqueKey = boundedKey; + for (let index = 2; uniqueKey in result; index += 1) { + uniqueKey = `${boundedKey}#${index}`; + } budget.remainingStringCharacters = Math.max( 0, budget.remainingStringCharacters - boundedKey.length ); try { - result[boundedKey] = boundedCanonicalToolValue( + result[uniqueKey] = boundedCanonicalToolValue( item, budget, depth + 1, nestedAncestors ); } catch { - result[boundedKey] = "[Unreadable value]"; + result[uniqueKey] = "[Unreadable value]"; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@contracts/chatCanonicalUtilities.ts` around lines 143 - 168, Update the object-property handling loop around boundedKey so truncated or provider-supplied keys cannot overwrite an existing result entry: when the bounded key already exists, generate a deterministic distinct key before assigning the value. Preserve the existing "[truncated]" marker behavior while ensuring a provider key with that name is also disambiguated.backend/src/gateway.ts (2)
771-781: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the duplicated timestamp validation into one shared helper.
gatewayTimestampinbackend/src/gateway.tsandtoTimestampinbackend/src/services/agents.tsimplement the same absolute-range timestamp check, including the same8_640_000_000_000_000bound. Move this logic intobackend/src/lib/values.ts(alongsidestringFallback) and import it from both files, so future changes to the validation rule do not need to be applied twice.
backend/src/gateway.ts#L771-L781: replace the localgatewayTimestampfunction body with a call to the shared helper.backend/src/services/agents.ts#L480-L490: replace the localtoTimestampfunction body with a call to the shared helper.♻️ Proposed shared helper
// backend/src/lib/values.ts export function boundedTimestamp(value: unknown): number | undefined { let timestamp = Number.NaN; if (typeof value === "number" && Number.isFinite(value)) { timestamp = value; } else if (typeof value === "string" && value.trim().length > 0) { timestamp = Date.parse(value); } return Number.isFinite(timestamp) && Math.abs(timestamp) <= 8_640_000_000_000_000 ? timestamp : undefined; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/gateway.ts` around lines 771 - 781, Extract the shared timestamp parsing and absolute-range validation into a new boundedTimestamp helper alongside stringFallback in backend/src/lib/values.ts. Update backend/src/gateway.ts lines 771-781 so gatewayTimestamp delegates to boundedTimestamp, and backend/src/services/agents.ts lines 480-490 so toTimestamp delegates to it; import the helper in both files and preserve the documented handling of finite numbers, non-empty strings, and the 8_640_000_000_000_000 bound.
752-853: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winSlice raw arrays before mapping, not after, to bound processing cost.
thinkingLevelsandthinkingOptionsingatewaySessionFromRecordrun.flatMap/.filter/.mapover the entire incoming array before.slice(0, 100)truncates the result. If the Gateway (or a compromised/misbehaving connection) sends an array with a very large number of entries, the function still processes every entry before truncating. Since this function exists specifically to bound untrusted Gateway session data, cap the raw array length first.♻️ Proposed fix to bound processing before mapping
const thinkingLevels = Array.isArray(record.thinkingLevels) ? record.thinkingLevels + .slice(0, 1000) .flatMap((value) => { const level = asRecord(value); const id = level ? gatewayString(level, "id")?.trim() : undefined; const label = level ? gatewayString(level, "label")?.trim() : undefined; return id && label ? [{ id, label }] : []; }) .slice(0, 100) : undefined; const thinkingOptions = Array.isArray(record.thinkingOptions) ? record.thinkingOptions + .slice(0, 1000) .filter((value): value is string => typeof value === "string") .map((value) => value.trim()) .filter(Boolean) .slice(0, 100) : undefined;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/gateway.ts` around lines 752 - 853, Update gatewaySessionFromRecord so thinkingLevels and thinkingOptions cap their input arrays to the first 100 elements before flatMap, filter, or map processing. Preserve the existing validation, trimming, and final output behavior while ensuring oversized untrusted arrays cannot incur processing beyond the bounded prefix.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/src/releaseManifest.ts`:
- Line 39: Ensure MANAGED_DASHBOARD_RUNTIME_LAUNCHER_ARTIFACT is executable
after release staging by either adding an activation test that asserts mode 0755
for the staged launcher or explicitly setting that mode during activation.
Preserve the existing staging behavior for systemd units.
In `@backend/src/services/dockerUpdater.ts`:
- Around line 124-136: Update managedComposePath and its callers to close the
check-use race by opening the validated compose file with a guarded handle,
retaining protection through dirtyDockerUpdaterPaths, fs.readFileSync, and
Docker Compose execution. Revalidate the handle’s regular-file, single-link,
canonical-root, and unchanged-identity properties immediately before reading,
and pass only the protected handle or its trusted path to Compose; ensure
cleanup releases the handle on every path.
In `@backend/src/services/gitHygiene.ts`:
- Around line 104-115: Update gitEnvironment() to delete the inherited
GIT_CONFIG_PARAMETERS entry before assigning GIT_CONFIG_COUNT, GIT_CONFIG_KEY_0,
and GIT_CONFIG_VALUE_0, ensuring Git subprocesses only receive the intended
configuration pairing.
In `@backend/test/serviceBehavior.test.ts`:
- Line 6318: Await the promise-based assertion in the test by updating the
expect(boundedRequest).resolves.toEqual call, ensuring the Bun test runner waits
for the assertion and reports rejections as test failures.
In `@contracts/chatCanonicalMessage.ts`:
- Around line 616-639: Update the array branch around truncateCanonicalChatText
to accumulate normalized blocks incrementally and stop once
MAX_CANONICAL_CHAT_TEXT_CHARACTERS is reached, avoiding an unbounded
map/filter/join intermediate string. Extract the per-item conversion into
canonicalChatTextBlock(item: unknown): string and reuse it for each block,
preserving the existing text, image, and ignored-value mappings.
- Around line 345-364: Update mergeCanonicalChatImages to derive deduplication
identity from the complete image data, including source.data, instead of
summarizeCanonicalChatValueForFingerprint(normalized). Preserve bounded hot-path
fingerprinting while ensuring distinct long base64 payloads with matching edge
samples are not treated as duplicates.
In `@docs/setup/production-deploy.md`:
- Around line 254-266: Update the release readiness comparison in the attempt
loop to require an exact match between current_commit from release-manifest.json
and expected, rather than prefix-matching with expected*. Preserve the existing
retry and failure behavior while enforcing the full SHA contract.
In `@frontend/src/components/features/database/DatabaseSizesTable.tsx`:
- Around line 36-54: Update mergeWithPoolData’s pool metric aggregation to
coerce each current and previous metric value to a finite number before
addition, preventing NaN from propagating through poolMap. Apply this
consistently to cl_active, cl_waiting, sv_active, sv_idle, and sv_used while
preserving the existing stringified sum output and fallback behavior.
In `@frontend/src/pages/Delivery.tsx`:
- Around line 936-944: Update refreshDelivery so refetchPullRequests always runs
even when reconcilePullRequestPreview.mutateAsync fails, while preserving the
existing reconciliation error reporting through setActionError. Use a
finally-style flow around the reconciliation attempt and keep the successful
refresh behavior unchanged.
---
Outside diff comments:
In `@backend/test/chatCanonicalMessage.test.ts`:
- Around line 15-37: Validate provider-supplied attachment URLs in the flow used
by AttachmentPreviewContent and chatImageDisplayUrl before rendering them as img
src or using them for downloads. Accept only data URLs or URLs from trusted
dashboard origins, rejecting arbitrary absolute or untrusted URLs; ensure the
existing canonicalChatImageDisplayUrl and related normalization paths enforce
this contract consistently.
In `@frontend/src/components/features/chat/chatTypes.ts`:
- Around line 297-461: Update chatImageDisplayUrl so data:image/svg+xml URLs
bypass the raster-only isEmbeddedChatImageWithinDimensionLimit check, while PNG,
GIF, JPEG, and WebP continue using it; retain any existing SVG MIME handling and
apply SVG-specific size validation if required. Add a regression test covering
an embedded base64 SVG data URL that successfully passes through
chatImageDisplayUrl.
---
Nitpick comments:
In `@backend/src/gateway.ts`:
- Around line 771-781: Extract the shared timestamp parsing and absolute-range
validation into a new boundedTimestamp helper alongside stringFallback in
backend/src/lib/values.ts. Update backend/src/gateway.ts lines 771-781 so
gatewayTimestamp delegates to boundedTimestamp, and
backend/src/services/agents.ts lines 480-490 so toTimestamp delegates to it;
import the helper in both files and preserve the documented handling of finite
numbers, non-empty strings, and the 8_640_000_000_000_000 bound.
- Around line 752-853: Update gatewaySessionFromRecord so thinkingLevels and
thinkingOptions cap their input arrays to the first 100 elements before flatMap,
filter, or map processing. Preserve the existing validation, trimming, and final
output behavior while ensuring oversized untrusted arrays cannot incur
processing beyond the bounded prefix.
In `@backend/src/services/databaseOverview.ts`:
- Around line 424-442: Update the torrent-count result handling around
countFromResult so rejected Promise.allSettled results are logged with their
rejection reason before returning 0. Preserve the existing fulfilled-result
parsing and zero fallback, and use the surrounding database overview logging
mechanism.
In `@backend/test/openClawChatBridge.test.ts`:
- Line 385: Update the retention assertion in openClawChatBridge.test.ts to
compare itemToolCount directly against the bridge’s configured retention-limit
constant, rather than only itemEventCount. Export that retention limit from the
bridge module if necessary, import it in the test, and assert the retained count
does not exceed it.
In `@contracts/chatCanonicalUtilities.ts`:
- Around line 143-168: Update the object-property handling loop around
boundedKey so truncated or provider-supplied keys cannot overwrite an existing
result entry: when the bounded key already exists, generate a deterministic
distinct key before assigning the value. Preserve the existing "[truncated]"
marker behavior while ensuring a provider key with that name is also
disambiguated.
In `@frontend/src/pages/Delivery.tsx`:
- Around line 1468-1472: Update the Delivery page refresh flow around
RefreshButton and usePullRequests so its loading state remains active while
either reconcilePullRequestPreview is pending or pull-request refetching is in
progress. Destructure and reuse the hook’s isFetching state, and include
isActionPending in the disabled/loading guard consistently with the other header
controls.
🪄 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 Plus
Run ID: 0ddd2d1c-0ec1-4e64-b4c1-2af23db63bc2
📒 Files selected for processing (71)
.github/workflows/codeql.yml.github/workflows/dashboard-checks.ymlbackend/src/development/developmentOpenClaw.tsbackend/src/gateway.tsbackend/src/health.tsbackend/src/http.tsbackend/src/lib/logTail.tsbackend/src/lib/openclawGatewayClient.tsbackend/src/lib/values.tsbackend/src/managedDashboardUnitPolicy.tsbackend/src/releaseDeployment.tsbackend/src/releaseManifest.tsbackend/src/requestPolicy.tsbackend/src/routes/cacheRoutes.tsbackend/src/routes/cronRoutes.tsbackend/src/routes/openclawConfigRoutes.tsbackend/src/routes/pullRequestRoutes.tsbackend/src/routes/taskRoutes.tsbackend/src/services/agents.tsbackend/src/services/backups.tsbackend/src/services/cacheRefresh.tsbackend/src/services/databaseOverview.tsbackend/src/services/dockerUpdater.tsbackend/src/services/gitHygiene.tsbackend/src/services/pullRequests.tsbackend/src/services/quotaNotifications.tsbackend/test/chatCanonicalMessage.test.tsbackend/test/databaseOverview.test.tsbackend/test/developmentStack.test.tsbackend/test/dockerUpdater.test.tsbackend/test/gatewayBehavior.test.tsbackend/test/gitHygiene.test.tsbackend/test/healthReadiness.test.tsbackend/test/httpApiBehavior.test.tsbackend/test/httpCookieBehavior.test.tsbackend/test/openClawChatBridge.test.tsbackend/test/releaseDeployment.test.tsbackend/test/releaseManager.test.tsbackend/test/releaseManifest.test.tsbackend/test/routeAndServiceBehavior.test.tsbackend/test/serviceBehavior.test.tsbackend/test/support/releaseFixture.tsbackend/test/utilityBehavior.test.tscontracts/chat/openClawAdapterValues.tscontracts/chat/openClawHistoryNormalizer.tscontracts/chat/openClawToolAdapter.tscontracts/chatCanonicalMessage.tscontracts/chatCanonicalUtilities.tscontracts/health.tscontracts/socket.tsdocs/setup/production-deploy.mdfrontend/src/collections/logs.tsfrontend/src/components/features/chat/chatPageUtilities.tsfrontend/src/components/features/chat/chatTypes.tsfrontend/src/components/features/chat/chatUtilities.tsfrontend/src/components/features/chat/domain/chatState.tsfrontend/src/components/features/database/DatabaseSizesTable.tsxfrontend/src/components/ui/Button.tsxfrontend/src/hooks/index.tsfrontend/src/hooks/useCron.tsfrontend/src/hooks/useDelivery.tsfrontend/src/hooks/useFileExplorerState.tsfrontend/src/pages/Delivery.tsxfrontend/src/test/chatCanonicalProjection.test.tsfrontend/src/test/chatState.test.tsfrontend/src/test/componentBehavior.test.tsxfrontend/src/test/frontendBehavior.test.tsxfrontend/src/test/openClawAdapterVariants.test.tsfrontend/src/test/pageBehavior.test.tsxsystemd/mira-dashboard-worker.servicesystemd/mira-dashboard.service
💤 Files with no reviewable changes (2)
- backend/src/health.ts
- contracts/health.ts
📜 Review details
🧰 Additional context used
🪛 OpenGrep (1.26.0)
frontend/src/components/features/chat/chatTypes.ts
[ERROR] 303-303: Dynamic command passed to child_process.exec/execSync. Use child_process.execFile or spawn with an argument array instead.
(coderabbit.command-injection.exec-js)
contracts/chatCanonicalMessage.ts
[ERROR] 367-367: Dynamic command passed to child_process.exec/execSync. Use child_process.execFile or spawn with an argument array instead.
(coderabbit.command-injection.exec-js)
🔇 Additional comments (76)
backend/test/developmentStack.test.ts (1)
330-343: LGTM!backend/test/dockerUpdater.test.ts (2)
2-9: LGTM!
255-295: LGTM!backend/src/lib/logTail.ts (1)
13-14: LGTM!Also applies to: 87-87, 105-118
backend/src/http.ts (1)
267-273: LGTM!Also applies to: 275-310, 312-347
backend/src/requestPolicy.ts (1)
627-639: LGTM!backend/src/routes/cacheRoutes.ts (1)
48-55: LGTM!Also applies to: 67-74, 123-130, 164-172, 233-239
backend/src/routes/pullRequestRoutes.ts (1)
81-81: LGTM!Also applies to: 263-278
backend/src/routes/taskRoutes.ts (1)
23-23: LGTM!Also applies to: 306-328
backend/src/services/backups.ts (1)
1171-1186: LGTM!backend/src/services/quotaNotifications.ts (1)
185-186: LGTM!backend/test/routeAndServiceBehavior.test.ts (1)
29-29: LGTM!Also applies to: 1002-1042, 3245-3254, 3650-3653, 5306-5327, 5645-5652, 5683-5685
backend/test/httpCookieBehavior.test.ts (1)
5-10: LGTM!Also applies to: 36-48
backend/test/httpApiBehavior.test.ts (1)
378-380: 🎯 Functional CorrectnessNo change needed.
The
unauthenticatedApitype literal has a singlechecksdeclaration, so there is no duplicate property error to address.> Likely an incorrect or invalid review comment.backend/src/services/cacheRefresh.ts (4)
1-3: LGTM!
1399-1435: LGTM!
1512-1512: LGTM!Also applies to: 1526-1526, 1539-1579
2324-2340: LGTM!backend/src/services/databaseOverview.ts (1)
444-450: LGTM!Also applies to: 543-547, 658-660
backend/test/utilityBehavior.test.ts (1)
474-488: LGTM!Also applies to: 587-587, 1354-1377, 1489-1489
backend/test/databaseOverview.test.ts (1)
263-266: LGTM!Also applies to: 313-318
backend/test/healthReadiness.test.ts (1)
49-50: LGTM!frontend/src/hooks/index.ts (1)
70-70: LGTM!frontend/src/hooks/useCron.ts (1)
49-56: LGTM!Also applies to: 73-79, 95-98, 114-117
frontend/src/hooks/useDelivery.ts (1)
243-248: LGTM!Also applies to: 318-332
frontend/src/pages/Delivery.tsx (1)
53-53: LGTM!Also applies to: 709-718, 889-889, 902-903
frontend/src/test/frontendBehavior.test.tsx (2)
48-48: LGTM!Also applies to: 3462-3481, 4695-4700, 4736-4736, 6217-6229, 6487-6505
6617-6628: 🎯 Functional CorrectnessNo change needed. This PNG branch only checks the first four PNG bytes and reads the widths at offsets 16 and 20, so the remaining zero signature bytes do not prevent the oversized-width assertion from exercising the dimension parser.
> Likely an incorrect or invalid review comment.contracts/chatCanonicalUtilities.ts (1)
43-76: LGTM!Also applies to: 176-186
contracts/chatCanonicalMessage.ts (1)
7-11: LGTM!Also applies to: 69-80, 215-215, 235-235, 366-421, 498-516, 546-549, 578-584
contracts/chat/openClawAdapterValues.ts (1)
4-8: LGTM!Also applies to: 54-75
contracts/chat/openClawHistoryNormalizer.ts (1)
18-22: LGTM!Also applies to: 381-384
contracts/chat/openClawToolAdapter.ts (2)
6-11: LGTM!Also applies to: 115-118
75-77: 🗄️ Data Integrity & IntegrationNo change needed for id-less tool IDs.
stableCanonicalChatStringifydoes not apply node/string budgets before keying; budget truncation has already replaced oversizedarguments_. Duplicates are handled by occurrence keys later, so this does not merge distinct id-less tool rows.> Likely an incorrect or invalid review comment.backend/test/chatCanonicalMessage.test.ts (1)
9-12: LGTM!Also applies to: 62-63, 81-132
backend/test/openClawChatBridge.test.ts (1)
8-8: LGTM!Also applies to: 350-351, 1796-1796, 1828-1838
frontend/src/test/openClawAdapterVariants.test.ts (1)
1496-1496: LGTM!frontend/src/test/componentBehavior.test.tsx (1)
536-536: LGTM!Also applies to: 4781-4804, 4821-4821
frontend/src/test/pageBehavior.test.tsx (1)
5652-5665: LGTM!frontend/src/collections/logs.ts (1)
7-9: LGTM!Also applies to: 61-72
frontend/src/components/features/chat/chatPageUtilities.ts (1)
27-36: LGTM!Also applies to: 46-66, 92-95
frontend/src/components/features/chat/chatTypes.ts (1)
6-10: LGTM!Also applies to: 20-22, 264-295, 506-506, 689-689
frontend/src/components/features/chat/chatUtilities.ts (1)
785-785: LGTM!frontend/src/components/features/chat/domain/chatState.ts (1)
5-9: LGTM!Also applies to: 29-29, 754-798, 812-815
frontend/src/components/ui/Button.tsx (1)
30-30: LGTM!frontend/src/hooks/useFileExplorerState.ts (1)
9-9: LGTM!frontend/src/test/chatCanonicalProjection.test.ts (1)
918-921: LGTM!Also applies to: 1001-1009, 1119-1132
frontend/src/test/chatState.test.ts (1)
3-11: LGTM!Also applies to: 820-869
.github/workflows/codeql.yml (1)
27-37: LGTM!.github/workflows/dashboard-checks.yml (1)
25-30: LGTM!Also applies to: 50-50, 59-59, 78-83, 101-101, 110-110
backend/src/managedDashboardUnitPolicy.ts (1)
16-17: LGTM!backend/src/releaseDeployment.ts (1)
19-19: LGTM!Also applies to: 258-260
backend/src/releaseManifest.ts (1)
22-25: LGTM!systemd/mira-dashboard-worker.service (1)
12-12: LGTM!backend/test/releaseDeployment.test.ts (1)
148-148: LGTM!Also applies to: 173-173
backend/test/releaseManifest.test.ts (1)
69-76: LGTM!Also applies to: 225-225
backend/src/lib/values.ts (1)
43-44: LGTM!backend/src/services/pullRequests.ts (1)
189-190: LGTM!Also applies to: 1045-1045, 1927-1927, 1944-1953, 3010-3015, 4271-4296
systemd/mira-dashboard.service (1)
12-12: LGTM!backend/test/releaseManager.test.ts (1)
154-161: LGTM!backend/test/support/releaseFixture.ts (1)
33-40: LGTM!backend/test/serviceBehavior.test.ts (1)
206-208: LGTM!Also applies to: 243-249, 713-715, 1031-1033, 2863-2863, 3413-3413, 3572-3576, 5365-5365, 8483-8487, 8962-8967, 8980-8989, 9079-9082
backend/src/services/dockerUpdater.ts (1)
89-102: LGTM!Also applies to: 571-585, 1687-1692, 1728-1728, 1765-1769, 1851-1852, 1892-1900, 2859-2860
backend/src/services/gitHygiene.ts (1)
34-34: LGTM!Also applies to: 243-243, 299-299, 531-532
backend/src/gateway.ts (1)
10-10: LGTM!Also applies to: 47-48, 862-880, 919-924, 1307-1313
backend/src/lib/openclawGatewayClient.ts (2)
24-38: LGTM!Also applies to: 742-756
126-127: 🩺 Stability & AvailabilityNo remaining
shouldWaitIndefinitelyusage left.contracts/socket.ts (1)
14-23: LGTM!backend/src/services/agents.ts (1)
1587-1587: LGTM!backend/test/gatewayBehavior.test.ts (1)
342-363: LGTM!Also applies to: 1060-1060
backend/src/routes/openclawConfigRoutes.ts (2)
96-129: LGTM!
131-144: 🎯 Functional CorrectnessConfirm hard-linked
SKILL.mdfiles are not a realistic packaging pattern.
isBoundedSkillManifestrequiresnlink === 1, so anySKILL.mdwith more than one hard link is excluded and drops the entire skill from/api/skillswhencollectSkillDirectories()filters out the directory. If OpenClaw skill installation or package tooling can create hard-linked manifests, change the check or raise a visible error instead of silently skipping valid skills.backend/src/routes/cronRoutes.ts (2)
112-126: LGTM!Also applies to: 139-144, 156-157, 169-169, 183-184, 195-198
33-59: 📐 Maintainability & Code QualityNo change needed.
backend/src/routes/taskRoutes.tsdefinescronJobId(job: CronJob), which is not the same parameter-validation function ascronJobId(request).> Likely an incorrect or invalid review comment.backend/src/development/developmentOpenClaw.ts (1)
75-76: LGTM!backend/test/gitHygiene.test.ts (1)
104-112: LGTM!
Review follow-up (
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/src/releaseManager.ts`:
- Around line 1239-1241: Update publishVerifiedDashboardRelease() so the
existing-release path repairs the execute bit on
scripts/runManagedDashboardRelease.sh before returning
loadManagedReleaseFromLayout(). Reuse the existing
MANAGED_DASHBOARD_RUNTIME_LAUNCHER_ARTIFACT and destination-path chmod behavior,
making the check idempotent while preserving the existing loader validation and
return flow.
In `@backend/src/services/agents.ts`:
- Line 1545: Update the session guard around boundedTimestamp so it explicitly
checks whether the returned endedAt value is undefined, preserving 0 as a valid
epoch timestamp and preventing such sessions from remaining marked as running.
🪄 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 Plus
Run ID: f091fa90-4ae7-423c-b280-f570816d866d
📒 Files selected for processing (21)
backend/src/gateway.tsbackend/src/lib/values.tsbackend/src/releaseManager.tsbackend/src/services/agents.tsbackend/src/services/databaseOverview.tsbackend/src/services/gitHygiene.tsbackend/test/chatCanonicalMessage.test.tsbackend/test/gatewayBehavior.test.tsbackend/test/gitHygiene.test.tsbackend/test/openClawChatBridge.test.tsbackend/test/releaseManager.test.tsbackend/test/serviceBehavior.test.tsbackend/test/utilityBehavior.test.tscontracts/chatCanonicalMessage.tscontracts/chatCanonicalUtilities.tsdocs/setup/production-deploy.mdfrontend/src/components/features/database/DatabaseSizesTable.tsxfrontend/src/pages/Delivery.tsxfrontend/src/test/componentBehavior.test.tsxfrontend/src/test/frontendBehavior.test.tsxfrontend/src/test/pageBehavior.test.tsx
🚧 Files skipped from review as they are similar to previous changes (15)
- backend/test/gitHygiene.test.ts
- backend/src/services/gitHygiene.ts
- backend/test/openClawChatBridge.test.ts
- backend/src/gateway.ts
- backend/test/utilityBehavior.test.ts
- docs/setup/production-deploy.md
- backend/test/serviceBehavior.test.ts
- contracts/chatCanonicalMessage.ts
- frontend/src/test/componentBehavior.test.tsx
- frontend/src/pages/Delivery.tsx
- backend/test/gatewayBehavior.test.ts
- contracts/chatCanonicalUtilities.ts
- frontend/src/components/features/database/DatabaseSizesTable.tsx
- frontend/src/test/frontendBehavior.test.tsx
- backend/test/releaseManager.test.ts
📜 Review details
🔇 Additional comments (10)
backend/test/chatCanonicalMessage.test.ts (1)
11-18: LGTM!Also applies to: 21-43, 69-69, 87-138, 140-197
frontend/src/test/pageBehavior.test.tsx (2)
5696-5709: 📐 Maintainability & Code QualityNo change needed. The existing assertions cover malformed storage clearing, legacy prompt-text cleanup, and opaque key rewriting.
3217-3242: 📐 Maintainability & Code QualityNo fetch cleanup issue remains.
The suite
beforeEachresetsglobalThis.fetchbefore each test, andafterEachrestoresoriginalGlobals.fetch, so this local mock cannot leak into the next test.backend/src/releaseManager.ts (1)
27-27: LGTM!backend/src/services/agents.ts (1)
25-25: LGTM!Also applies to: 475-475, 1510-1511, 1568-1570
backend/src/lib/values.ts (2)
138-154: LGTM!
43-44: 🎯 Functional CorrectnessNo caller changes needed.
backend/src/services/databaseOverview.ts (3)
10-10: LGTM!Also applies to: 24-24
430-444: LGTM!
551-555: 🎯 Functional CorrectnessThreshold rules are aligned.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 18e8c6bba6
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/src/releaseManager.ts`:
- Around line 1222-1239: Extract the existing launcher chmod operation into an
ensureManagedLauncherExecutable helper, and call it for both existingRelease and
concurrentlyPublished paths. Update the EEXIST/ENOTEMPTY collision branch to
repair concurrentlyPublished before returning it, preserving the current
release-return behavior.
🪄 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 Plus
Run ID: 828d2d57-96f6-497a-a1f6-61b1a589cc6b
📒 Files selected for processing (24)
backend/src/database.tsbackend/src/gatewayToken.tsbackend/src/releaseManager.tsbackend/src/routes/mediaRoutes.tsbackend/src/routes/pullRequestRoutes.tsbackend/src/serverStart.tsbackend/src/services/agents.tsbackend/src/services/pullRequestPreviewHost.tsbackend/src/services/pullRequestPreviews.tsbackend/src/services/pullRequests.tsbackend/test/chatCanonicalMessage.test.tsbackend/test/gatewayBehavior.test.tsbackend/test/pullRequestPreview.test.tsbackend/test/releaseManager.test.tsbackend/test/serverStartupPolicy.test.tsbackend/test/serviceBehavior.test.tsbackend/test/testDatabaseGuard.test.tscontracts/chat/openClawHistoryNormalizer.tscontracts/chat/openClawHistoryPageAdapter.tscontracts/chatCanonical.tscontracts/chatCanonicalMessage.tsfrontend/src/pages/Delivery.tsxfrontend/src/test/pageBehavior.test.tsxpackage.json
💤 Files with no reviewable changes (2)
- frontend/src/test/pageBehavior.test.tsx
- backend/src/routes/pullRequestRoutes.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- frontend/src/pages/Delivery.tsx
- backend/test/gatewayBehavior.test.ts
- backend/test/chatCanonicalMessage.test.ts
- backend/src/services/agents.ts
- backend/src/services/pullRequests.ts
- contracts/chatCanonicalMessage.ts
- backend/test/serviceBehavior.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: backend-checks
🔇 Additional comments (23)
backend/src/releaseManager.ts (1)
550-597: LGTM!Also applies to: 886-911, 1273-1275
backend/src/database.ts (1)
21-37: LGTM!Also applies to: 51-60, 99-99, 284-284, 308-308
backend/src/gatewayToken.ts (1)
1-18: LGTM!backend/src/routes/mediaRoutes.ts (1)
6-6: LGTM!Also applies to: 85-87
backend/src/serverStart.ts (1)
2-2: LGTM!backend/src/services/pullRequestPreviewHost.ts (1)
31-31: LGTM!Also applies to: 1706-1709
backend/src/services/pullRequestPreviews.ts (3)
6-6: LGTM!Also applies to: 31-36, 46-47
245-276: LGTM!
371-379: LGTM!Also applies to: 437-467
contracts/chatCanonical.ts (1)
8-17: LGTM!Also applies to: 31-31
contracts/chat/openClawHistoryNormalizer.ts (3)
12-12: LGTM!
369-387: LGTM!
415-440: LGTM!contracts/chat/openClawHistoryPageAdapter.ts (3)
1-4: LGTM!
57-67: LGTM!
99-108: LGTM!package.json (1)
15-16: LGTM!backend/test/pullRequestPreview.test.ts (3)
527-528: 🩺 Stability & AvailabilityVerify token restoration runs even if an earlier assertion fails.
The lifecycle test sets
OPENCLAW_GATEWAY_TOKENat line 528, switchesexpectedUpstreamTokenat line 771-772, and restores the original value at lines 1031-1035. This span covers many assertions between the mutation and the restoration.If an assertion between line 528 and line 1031 throws, the restoration step does not run. The modified environment variable then leaks into later tests in the same process, causing unrelated test failures or flaky passes.
Confirm that this restoration executes inside a
try/finallyblock or anafterEachhook. If it does not, wrap the token mutation and restoration intry/finally.Also applies to: 543-543, 632-632, 771-772, 1031-1035
1344-1373: LGTM!
1283-1289: 🎯 Functional CorrectnessNo change needed for these
.resolves/.rejectsmatchers.Bun’s native test matchers do not require
awaiton the assertion expression; addingawaithere is optional style, not a functional fix.> Likely an incorrect or invalid review comment.backend/test/releaseManager.test.ts (1)
365-390: LGTM!Also applies to: 521-547
backend/test/serverStartupPolicy.test.ts (1)
70-72: LGTM!backend/test/testDatabaseGuard.test.ts (1)
1-1: LGTM!Also applies to: 104-139, 184-226
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 00b173ae57
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 96df63c23e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
2 participants
Summary
Finding disposition
GitHub's preview stack-merge API can pin only the selected PR SHA. This branch restricts every stack layer to configured trusted authors and revalidates every expected head immediately before merge; lower-layer atomic SHA pinning remains a documented upstream API limitation.
No old-database migration path or obsolete-browser compatibility layer was added.
Preserved behavior
tasks:writecan still create and update tasks and trigger Mira notifications, but automation-originated notification text contains only trusted event metadataVerification
bun run test:frontend— 663 passed, 0 failedbun run test:backend— 708 passed, 0 failedbun run buildbun run lintbun run format:checkgit diff --checkDashboard task: #387