fix(reviewers): add uptime context and review root settings#8
Conversation
The getServerRuntimeContext() signature changed to accept pre-computed uptime in ms (defaulting to process.uptime() * 1000), but the call site in buildContextBlock() was still passing now.getTime() — a Date.now() epoch value (~1.78e12 ms). This caused the system prompt to report a multi-decade server uptime (e.g. "20600d 0h") instead of the actual process uptime. Fix: call getServerRuntimeContext() with no arguments to use the default. Also add a regression test that rejects epoch-scale uptime values. Addresses kilo-code-bot review comment on PR #7.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
PR SummaryMedium Risk Overview Introduces configurable repository containment for local CodeRabbit/Kilo review: paths are realpath-resolved, must sit under the tool workspace or Reviewer Agents API validation now accepts remediation targets with optional Reviewed by Cursor Bugbot for commit a7698ce. Configure here. |
|
Warning Review limit reached
Next review available in: 2 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: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (30)
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 |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_586477f6-0608-4113-b196-ce1099f00cad) |
![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 introduces repository containment security for local review tools, allowing administrators to configure allowed host repository roots outside the workspace via a new settings page. It also adds server process uptime context to the system prompt. The code review feedback highlights several key areas for improvement: a security vulnerability where parent Git repositories could allow containment escape, a potential server crash on invalid timezone configurations, an inaccurate server start time calculation, a client-server path validation mismatch on Windows, and the inclusion of untranslated English strings in non-English locale files.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| const gitTopLevel = gitTopLevelForRepo(repoRealPath) | ||
| if (!gitTopLevel || !isPathInsideOrEqual(gitTopLevel, repoRealPath)) { | ||
| throw new Error(`repo_path must be a Git repository root or contain a Git worktree: ${repoPath}`) | ||
| } |
There was a problem hiding this comment.
Security Vulnerability: Containment Escape via Git Parent Repository
If a parent directory of the allowed root or workspace is a Git repository, git -C will traverse up to find it. Since gitTopLevel is not validated against the allowed roots, any Git operations executed by the local review tools can access and expose files outside the allowed containment boundary (e.g., reading sensitive files in the parent Git repository).
To prevent this containment escape, we must ensure that the resolved gitTopLevel is also inside or equal to either the workspace root or one of the configured allowed roots.
const gitTopLevel = gitTopLevelForRepo(repoRealPath)
if (!gitTopLevel || !isPathInsideOrEqual(gitTopLevel, repoRealPath)) {
throw new Error('repo_path must be a Git repository root or contain a Git worktree: ' + repoPath)
}
const gitTopLevelAllowed = [workspaceRealPath, ...allowedRoots].some((root) => isPathInsideOrEqual(root, gitTopLevel))
if (!gitTopLevelAllowed) {
throw new Error('Git repository root must resolve inside the current tool workspace/worktree or a configured code-review allowed root')
}| const runtime = getServerRuntimeContext() | ||
| const serverStarted = runtime.startedAt.toLocaleString('en-US', { | ||
| year: 'numeric', | ||
| month: 'long', | ||
| day: 'numeric', | ||
| hour: '2-digit', | ||
| minute: '2-digit', | ||
| second: '2-digit', | ||
| timeZone: tz, | ||
| timeZoneName: 'short', | ||
| }) |
There was a problem hiding this comment.
Robustness: Potential Crash on Invalid Timezone Configuration
If the user configures an invalid timezone in HIVEKEEP_TIMEZONE or TZ, toLocaleString will throw a RangeError: Invalid time zone specified, crashing the prompt builder and preventing the agent from starting or responding.
We should wrap this in a try-catch block to gracefully fall back to 'UTC' if the configured timezone is invalid.
const runtime = getServerRuntimeContext()
let serverStarted = ''
try {
serverStarted = runtime.startedAt.toLocaleString('en-US', {
year: 'numeric',
month: 'long',
day: 'numeric',
hour: '2-digit',
minute: '2-digit',
second: '2-digit',
timeZone: tz,
timeZoneName: 'short',
})
} catch {
serverStarted = runtime.startedAt.toLocaleString('en-US', {
year: 'numeric',
month: 'long',
day: 'numeric',
hour: '2-digit',
minute: '2-digit',
second: '2-digit',
timeZone: 'UTC',
timeZoneName: 'short',
})
}| @@ -0,0 +1,40 @@ | |||
| export const SERVER_STARTED_AT = Date.now() | |||
There was a problem hiding this comment.
Correctness: Inaccurate Process Start Time
SERVER_STARTED_AT is initialized to Date.now() when the module is loaded, which can be slightly after the actual process start time due to module loading delays.
To accurately represent the process start time, we should subtract the process uptime from the current time.
| export const SERVER_STARTED_AT = Date.now() | |
| export const SERVER_STARTED_AT = Date.now() - process.uptime() * 1000 |
| function isAbsolutePath(value: string): boolean { | ||
| return value.startsWith('/') || /^[A-Za-z]:[\\/]/.test(value) | ||
| } |
There was a problem hiding this comment.
Correctness: Platform-Dependent Path Validation Mismatch
On Windows, paths starting with a single slash (e.g., /foo) are considered drive-relative and not fully absolute. Node's path.isAbsolute('/foo') returns false on Windows, whereas the client-side isAbsolutePath helper returns true because of value.startsWith('/').
This causes a validation mismatch where the client allows saving /foo on Windows, but the server subsequently rejects it with a 400 Bad Request error.
| "codeReview": { | ||
| "title": "Code Review", | ||
| "description": "Configure host repository roots that local CodeRabbit and Kilo reviewer tools may access outside the current Agent workspace.", |
There was a problem hiding this comment.
Internationalization (i18n): Untranslated English Strings in Non-English Locales
Adding untranslated English strings directly to non-English locale files (such as de.json, es.json, fr.json, it.json, ja.json, pl.json, pt-BR.json, ru.json, zh-CN.json) violates internationalization best practices. It pollutes translation files and makes it difficult for translation tools or contributors to identify which keys actually need translation.
Instead, these keys should only be added to en.json and omitted from non-English files so they can gracefully fall back to the English locale via i18next's fallback mechanism.
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| "source": "Source: {{source}}", | ||
| "sourceSettings": "Settings override", | ||
| "sourceEnv": "environment fallback", | ||
| "rootPlaceholder": "/Users/kdegeek/hivekeep", |
There was a problem hiding this comment.
WARNING: Developer-specific home path shipped as a placeholder to all users in all 10 locales.
/Users/kdegeek/hivekeep is the macOS home directory of the PR author (matches kdegeek <nate@kdegeek.com> in the commit log). It is duplicated byte-for-byte into de.json, es.json, fr.json, it.json, ja.json, pl.json, pt-BR.json, ru.json, and zh-CN.json, so every install on every OS sees this path as the example. On non-macOS hosts /Users/... is invalid, and the literal leak is avoidable.
Use a generic, OS-agnostic example such as /path/to/repo-root or /home/me/repos.
| "rootPlaceholder": "/Users/kdegeek/hivekeep", | |
| "rootPlaceholder": "/path/to/repo-root", |
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| </p> | ||
| </div> | ||
|
|
||
| <Alert className="border-amber-500/30 bg-amber-500/5 text-amber-900 dark:text-amber-100"> |
There was a problem hiding this comment.
SUGGESTION: Hardcoded amber Tailwind colors bypass the 18-palette design system.
The security Alert uses literal border-amber-500/30 bg-amber-500/5 text-amber-900 dark:text-amber-100 instead of semantic tokens. Per project rule #3, UI must look correct across all 18 palettes (aurora, ocean, forest, sunset, monochrome, sakura, neon, lavender, midnight, copper, jade, crimson, galaxy, amber, slate, rose, mint, citrus) in both light and dark modes; this Alert will always render amber regardless of the active palette. Use a semantic token (e.g. text-warning / border-warning) or a palette-aware Alert variant so the security callout respects the user's chosen palette.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Code Review SummaryStatus: 2 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (30 files)
Fix these issues in Kilo Cloud Reviewed by minimax-m3 · Input: 113K · Output: 17.8K · Cached: 986.2K |
1 participant
Fixes the remaining review comment from PR #7.
Changes
src/server/services/prompt-builder.ts: FixedgetServerRuntimeContext()call site that was passingnow.getTime()(epoch milliseconds ≈ 1.78e12) as theuptimeMsparameter after the signature changed to expect pre-computed uptime in ms. This caused the system prompt to report a multi-decade server uptime (e.g. "20600d 0h") instead of the actual process uptime. Now callsgetServerRuntimeContext()with no arguments to use the defaultprocess.uptime() * 1000.src/server/services/prompt-builder.test.ts: Added a regression test that rejects epoch-scale uptime values (days > 1), which would have caught the original bug.Addresses kilo-code-bot review comment on PR #7.