Skip to content

feat(api): env-configurable box image allowlist - #1027

Merged
DorianZheng merged 1 commit into
mainfrom
worktree-distributed-dancing-taco
Jul 24, 2026
Merged

feat(api): env-configurable box image allowlist#1027
DorianZheng merged 1 commit into
mainfrom
worktree-distributed-dancing-taco

Conversation

@DorianZheng

@DorianZheng DorianZheng commented Jul 24, 2026

Copy link
Copy Markdown
Member

What

Make the box-image allowlist configurable from env so images can be added
without a code deploy.

  • BOXLITE_SYSTEM_IMAGES (comma-separated name=ref) appends images to the
    built-in base/python/node set; base stays the default.
  • Images are { name, ref }; a create-time selector resolves by name or full
    OCI ref to the ref, so persist / warm-pool / runner stay on opaque refs.
  • A malformed name=ref entry throws at the request boundary (operator config).

Verification

  • Unit spec 9/9, incl. two-side A/B (old string[] code fails the new spec).
  • tsc --noEmit on the api app source clean; sole consumer box.service.ts:175
    keeps the same string return.
  • Live local-infra run (POST /api/v1/boxes): hermes accepted by name and by
    full ref (persisted as the resolved ref), unknowns rejected with the full
    supported list, omitted image defaults to base.

Note: actually booting sam2026go/hermes-agent:boxlite needs a linux/arm64
image build (currently amd64-only) — out of scope for this allowlist change.

https://claude.ai/code/session_01YFLhjZmRUvDUFtwBp9wELy

Summary by CodeRabbit

  • New Features

    • Select system images by built-in names or full OCI references when creating a box.
    • Add custom supported images through BOXLITE_SYSTEM_IMAGES using name=ref entries.
    • Configure image overrides through environment settings.
  • Bug Fixes

    • Improved validation and error messages for unsupported or malformed image selections.
  • Documentation

    • Updated image configuration guidance and API examples, including the default base image.

Add BOXLITE_SYSTEM_IMAGES so operators can extend the curated box-image
set (comma-separated `name=ref`) without a code deploy. Each image now
carries a short name and an OCI ref; a create-time selector resolves by
name or full ref to the ref, keeping persist / warm-pool / runner on
opaque refs. The built-in base/python/node refs stay env-overridable and
base remains the default. A malformed entry throws at the boundary.

Claude-Session: https://claude.ai/code/session_01YFLhjZmRUvDUFtwBp9wELy
@DorianZheng
DorianZheng requested a review from a team July 24, 2026 15:57
@boxlite-agent

boxlite-agent Bot commented Jul 24, 2026

Copy link
Copy Markdown

📦 BoxLite review — 1 issue · 9c36a31

Review evidence

  • git diff --numstat origin/main...HEAD && git diff origin/main...HEAD — 4 files changed: curated-images ts+spec, create-box.dto.ts, sst.config.ts
  • npx jest src/box/constants/curated-images.constant.spec.ts — nx jest preset missing, yarn install hit ENOSPC on tmpfs /tmp
  • node port of supportedImages/assertSupportedImage logic (verify.mjs) — confirmed builtin-name collision safe; malformed env throws for all lookups

Risk notes

  • security: image allowlist bypass — find() checks builtins before extras, so an operator-added name/ref can't shadow a builtin; unknown selectors still rejected with BadRequestError. Safe.
  • availability: operator env misconfig — BOXLITE_SYSTEM_IMAGES parse errors throw inside supportedImages(), which is called for every create-box request incl. default image — one bad entry outages box creation entirely (see finding).
  • api contract — assertSupportedImage now resolves name-or-ref to a ref; single call site in box.service.ts already treats result as opaque ref (warm-pool key, pull), so behavior downstream unaffected.
  • tests — spec.ts rewritten to cover name resolution, extras parsing/whitespace/dedup boundaries, and malformed-entry throw; matches new implementation, no coverage weakened.
  • sst.config.ts / infra — only adds envOr passthrough for BOXLITE_SYSTEM_IMAGES defaulting to '', trivial and consistent with existing pattern. Sampled, not deep-audited (infra file only 7/5 lines changed).
apps/api/src/box/constants/curated-images.constant.ts
  supportedImages/assertSupportedImage/parseExtraImages  +65/-20  name+ref selector, extra images via env
apps/api/src/box/constants/curated-images.constant.spec.ts
  describe(supported image allowlist)  +57/-17  rewritten tests for new selector API
apps/api/src/box/dto/create-box.dto.ts
  CreateBoxDto.image  +3/-2  doc/example only, no validation change
apps/infra/sst.config.ts
  Api env config  +7/-5  adds BOXLITE_SYSTEM_IMAGES envOr passthrough
1 finding summary
  • ⚠️ apps/api/src/box/constants/curated-images.constant.ts:48-60 Malformed BOXLITE_SYSTEM_IMAGES breaks all box creation — parseExtraImages throws inside supportedImages(), called on every assertSupportedImage() call, so one bad operator entry makes even the default 'base' lookup throw and box creation fully unavailable; repro'd with a standalone port: BOXLITE_SYSTEM_IMAGES='bad-entry-no-equals' -> assertSupportedImage('base') throws instead of returning the base ref.

reviewed 9c36a31 in a BoxLite microVM · @boxlite-agent review to re-run · powered by BoxLite

@cla-assistant

cla-assistant Bot commented Jul 24, 2026

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


tester seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@coderabbitai

coderabbitai Bot commented Jul 24, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

Failed to post review comments.

We encountered an issue with GitHub. Use @coderabbitai full review to retry the review.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 3cef0417-0f93-41bc-b403-bcd21a861980

📥 Commits

Reviewing files that changed from the base of the PR and between cca32d5 and 9c36a31.

📒 Files selected for processing (4)
  • apps/api/src/box/constants/curated-images.constant.spec.ts
  • apps/api/src/box/constants/curated-images.constant.ts
  • apps/api/src/box/dto/create-box.dto.ts
  • apps/infra/sst.config.ts
⏰ Context from checks skipped due to timeout. (8)
  • GitHub Check: Regenerate API clients and diff
  • GitHub Check: Analyze (javascript-typescript)
  • GitHub Check: Analyze (rust)
  • GitHub Check: Analyze (actions)
  • GitHub Check: Analyze (go)
  • GitHub Check: Analyze (javascript-typescript)
  • GitHub Check: Analyze (go)
  • GitHub Check: BoxLite review
🧰 Additional context used
📓 Path-based instructions (3)
apps/infra/**/*.{py,js,ts,tsx,java,go,rb,php}

📄 CodeRabbit inference engine (apps/infra/AGENTS.md)

apps/infra/**/*.{py,js,ts,tsx,java,go,rb,php}: Use Claude API for LLM interactions
Include error handling for API failures and rate limiting

Files:

  • apps/infra/sst.config.ts
apps/infra/**/*.{py,js,ts,tsx,json,yaml,yml}

📄 CodeRabbit inference engine (apps/infra/AGENTS.md)

Follow the Claude API documentation for authentication and request formats

Files:

  • apps/infra/sst.config.ts
apps/infra/**/{README,*.md,*.env*,*.config.*,*.conf}

📄 CodeRabbit inference engine (apps/infra/AGENTS.md)

Document API token management and security best practices

Files:

  • apps/infra/sst.config.ts
🧠 Learnings (1)
📚 Learning: 2026-06-29T04:50:08.549Z
Learnt from: law-chain-hot
Repo: boxlite-ai/boxlite PR: 861
File: apps/api/src/box/repositories/box.repository.ts:0-0
Timestamp: 2026-06-29T04:50:08.549Z
Learning: In this codebase, any proxy “desired-state” notification for boxes tied to `BoxEvents.DESIRED_STATE_UPDATED` (e.g., Redis publish and websocket fan-out) must be emitted only after the surrounding database transaction has committed. Do not perform or trigger these external I/O side-effects inside repository-layer transaction blocks (such as `apps/api/src/box/repositories/box.repository.ts`); instead, emit them post-commit from the service layer (e.g., `ensureStartedForProxy` in `apps/api/src/box/services/box.service.ts`). This prevents irreversible side-effects from firing on rollbacks.

Applied to files:

  • apps/api/src/box/dto/create-box.dto.ts
  • apps/api/src/box/constants/curated-images.constant.spec.ts
  • apps/api/src/box/constants/curated-images.constant.ts
📝 Walkthrough

Walkthrough

Curated image selection now supports built-in names, full OCI references, environment overrides, and operator-defined name=ref entries. The API documentation and infrastructure configuration expose the new image configuration contract, with tests covering parsing, resolution, defaults, and rejection behavior.

Changes

Curated image allowlisting

Layer / File(s) Summary
Image model, parsing, and resolution
apps/api/src/box/constants/curated-images.constant.ts
Introduces named image/reference pairs, parses BOXLITE_SYSTEM_IMAGES, resolves names or references, applies environment overrides, and defaults to the base image.
Selector contract validation
apps/api/src/box/constants/curated-images.constant.spec.ts
Tests built-in and extra-image resolution, reference passthrough, whitespace handling, malformed entries, defaults, overrides, and unsupported selectors.
API documentation and runtime configuration
apps/api/src/box/dto/create-box.dto.ts, apps/infra/sst.config.ts
Documents supported image selectors and injects BOXLITE_SYSTEM_IMAGES into the API environment.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

Suggested reviewers: law-chain-hot

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title is concise and clearly summarizes the main change to env-configurable box image allowlisting.
Description check ✅ Passed It covers the main change and verification details, but it does not follow the template headings exactly.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch worktree-distributed-dancing-taco

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.

❤️ Share

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

@DorianZheng
DorianZheng merged commit 53c7aea into main Jul 24, 2026
30 of 32 checks passed
@DorianZheng
DorianZheng deleted the worktree-distributed-dancing-taco branch July 24, 2026 16:00

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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 `@apps/api/src/box/constants/curated-images.constant.ts`:
- Around line 80-85: The supportedImages() allowlist currently permits duplicate
names, causing assertSupportedImage() to resolve the first match. After
combining builtins with parseExtraImages(), validate that every image name is
unique and reject duplicate built-in/custom and duplicate-extra entries;
preserve the existing returned image list for valid configuration.
🪄 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: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 3cef0417-0f93-41bc-b403-bcd21a861980

📥 Commits

Reviewing files that changed from the base of the PR and between cca32d5 and 9c36a31.

📒 Files selected for processing (4)
  • apps/api/src/box/constants/curated-images.constant.spec.ts
  • apps/api/src/box/constants/curated-images.constant.ts
  • apps/api/src/box/dto/create-box.dto.ts
  • apps/infra/sst.config.ts

Comment on lines +80 to +85
export function supportedImages(): SupportedImage[] {
const builtins = BUILTIN_IMAGE_SOURCES.map(({ name, envVar, fallbackRef }) => ({
name,
ref: process.env[envVar] || fallbackRef,
}))
return [...builtins, ...parseExtraImages(process.env[EXTRA_IMAGES_ENV])]

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Reject duplicate image names before returning the allowlist.

assertSupportedImage() uses find(), so BOXLITE_SYSTEM_IMAGES=base=... or two hermes=... entries silently resolve the first entry instead of failing configuration. Validate unique names after combining built-ins and extras; add duplicate built-in/custom and duplicate-extra tests.

Suggested fix
 export function supportedImages(): SupportedImage[] {
   const builtins = BUILTIN_IMAGE_SOURCES.map(({ name, envVar, fallbackRef }) => ({
     name,
     ref: process.env[envVar] || fallbackRef,
   }))
-  return [...builtins, ...parseExtraImages(process.env[EXTRA_IMAGES_ENV])]
+  const images = [...builtins, ...parseExtraImages(process.env[EXTRA_IMAGES_ENV])]
+  const names = new Set<string>()
+  for (const { name } of images) {
+    if (names.has(name)) {
+      throw new Error(`Duplicate supported image name '${name}'`)
+    }
+    names.add(name)
+  }
+  return images
 }
📝 Committable suggestion

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

Suggested change
export function supportedImages(): SupportedImage[] {
const builtins = BUILTIN_IMAGE_SOURCES.map(({ name, envVar, fallbackRef }) => ({
name,
ref: process.env[envVar] || fallbackRef,
}))
return [...builtins, ...parseExtraImages(process.env[EXTRA_IMAGES_ENV])]
export function supportedImages(): SupportedImage[] {
const builtins = BUILTIN_IMAGE_SOURCES.map(({ name, envVar, fallbackRef }) => ({
name,
ref: process.env[envVar] || fallbackRef,
}))
const images = [...builtins, ...parseExtraImages(process.env[EXTRA_IMAGES_ENV])]
const names = new Set<string>()
for (const { name } of images) {
if (names.has(name)) {
throw new Error(`Duplicate supported image name '${name}'`)
}
names.add(name)
}
return images
}
🤖 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 `@apps/api/src/box/constants/curated-images.constant.ts` around lines 80 - 85,
The supportedImages() allowlist currently permits duplicate names, causing
assertSupportedImage() to resolve the first match. After combining builtins with
parseExtraImages(), validate that every image name is unique and reject
duplicate built-in/custom and duplicate-extra entries; preserve the existing
returned image list for valid configuration.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant