feat(workspace): admin system prompt shared with all users - #2109
Conversation
Introduce SYSTEM.md in a well-known __admin__ scope so admins can set a system prompt that all tenants receive. Gated behind multi-tenant mode (WorkspacePool sets admin_prompt_enabled on each workspace; owner workspace in app.rs also gets the flag when has_any_users() is true). New endpoints: - GET /api/admin/system-prompt — read admin system prompt - PUT /api/admin/system-prompt — set admin system prompt (64 KB limit) Safety: - SYSTEM.md added to injection scan list - is_reserved_scope() guard on user creation (defense-in-depth) - Multi-tenancy gate on both API and prompt assembly layers Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
This pull request implements an admin system prompt feature for multi-tenant environments, enabling global instructions to be defined in a reserved "admin" scope and injected into all user prompts. Key additions include management API handlers, routing, and safety checks to prevent user ID collisions with system scopes. Feedback highlights the need for a content size limit on the system prompt to avoid token exhaustion or context overflow.
| pub async fn put_handler( | ||
| State(state): State<Arc<GatewayState>>, | ||
| AdminUser(_admin): AdminUser, | ||
| Json(req): Json<SystemPromptRequest>, | ||
| ) -> Result<Json<SystemPromptResponse>, (StatusCode, String)> { | ||
| // Gate behind multi-tenant mode. | ||
| if state.workspace_pool.is_none() { | ||
| return Err(( | ||
| StatusCode::NOT_FOUND, | ||
| "System prompt management requires multi-tenant mode".to_string(), | ||
| )); | ||
| } | ||
|
|
||
| let db = state.store.as_ref().ok_or(( | ||
| StatusCode::SERVICE_UNAVAILABLE, | ||
| "Database not available".to_string(), | ||
| ))?; | ||
|
|
||
| let ws = Workspace::new_with_db(ADMIN_SCOPE, Arc::clone(db)); | ||
|
|
||
| let doc = ws.write(paths::SYSTEM, &req.content).await.map_err(|e| { | ||
| let status = if matches!(e, crate::error::WorkspaceError::InjectionRejected { .. }) { | ||
| StatusCode::BAD_REQUEST | ||
| } else { | ||
| StatusCode::INTERNAL_SERVER_ERROR | ||
| }; | ||
| (status, e.to_string()) | ||
| })?; | ||
|
|
||
| Ok(Json(SystemPromptResponse { | ||
| content: doc.content, | ||
| updated_at: Some(doc.updated_at.to_rfc3339()), | ||
| })) | ||
| } |
There was a problem hiding this comment.
The put_handler lacks a content size limit for the system prompt. Since this content is injected into every user's system prompt, an unbounded size could lead to token budget exhaustion or context overflow. A 64 KB limit is recommended.
pub async fn put_handler(
State(state): State<Arc<GatewayState>>,
AdminUser(_admin): AdminUser,
Json(req): Json<SystemPromptRequest>,
) -> Result<Json<SystemPromptResponse>, (StatusCode, String)> {
if req.content.len() > 64 * 1024 {
return Err((StatusCode::PAYLOAD_TOO_LARGE, "System prompt exceeds 64 KB limit".to_string()));
}
// Gate behind multi-tenant mode.
if state.workspace_pool.is_none() {
return Err((
StatusCode::NOT_FOUND,
"System prompt management requires multi-tenant mode".to_string(),
));
}
let db = state.store.as_ref().ok_or((
StatusCode::SERVICE_UNAVAILABLE,
"Database not available".to_string(),
))?;
let ws = Workspace::new_with_db(ADMIN_SCOPE, Arc::clone(db));
let doc = ws.write(paths::SYSTEM, &req.content).await.map_err(|e| {
let status = if matches!(e, crate::error::WorkspaceError::InjectionRejected { .. }) {
StatusCode::BAD_REQUEST
} else {
StatusCode::INTERNAL_SERVER_ERROR
};
(status, e.to_string())
})?;
Ok(Json(SystemPromptResponse {
content: doc.content,
updated_at: Some(doc.updated_at.to_rfc3339()),
}))
}Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
henrypark133
left a comment
There was a problem hiding this comment.
Code Review Summary
| Severity | Count |
|---|---|
| Warning | 2 |
| Nit | 2 |
Rust safety: Clean — no unwraps in production paths, let-chain syntax valid for edition 2024 / MSRV 1.92, ADMIN_SCOPE write isolation correct.
Open Questions
- Is the owner-workspace asymmetry (warning 2) acceptable, or should it be documented before merge?
- Is
PUT ""the intended way to clear the system prompt? No DELETE endpoint, and not documented.
Test Coverage Notes
- 6 workspace-layer integration tests are well-structured and cover the critical behavioral paths.
- The 64 KB limit is untested because it's not implemented — needs a test once added.
- HTTP handler gate (single-user → 404) is not tested.
| } | ||
|
|
||
| /// `PUT /api/admin/system-prompt` — set the admin system prompt. | ||
| pub async fn put_handler( |
There was a problem hiding this comment.
[warning | Logic Completeness] The PR description states a "64 KB size limit" but this handler has no content-length check. An admin can write multi-MB content that is injected verbatim into every user's system prompt, silently exhausting token budgets.
gemini-code-assist already flagged this; the limit remains unimplemented.
Suggested fix — add at the start of put_handler:
if req.content.len() > 64 * 1024 {
return Err((StatusCode::PAYLOAD_TOO_LARGE, "System prompt exceeds 64 KB limit".to_string()));
}Also add a corresponding test case in tests/admin_system_prompt.rs.
There was a problem hiding this comment.
Done — added the 64 KB size limit at the top of put_handler (before the multi-tenant gate) and two regression tests:
put_rejects_oversized_system_prompt— verifies 413 for content > 64 KBput_accepts_system_prompt_within_limit— verifies content at exactly 64 KB passes the size check
See commit 4efa4cb.
| // workspace. Even outside authenticated multi-tenant mode, some | ||
| // channels and test harnesses route non-owner users through | ||
| // per-user tenant workspaces seeded on demand. | ||
| let is_multi_tenant = db.has_any_users().await.unwrap_or(false); |
There was a problem hiding this comment.
[warning | Behavioral Regression] is_multi_tenant is evaluated once at startup. If the server starts with no users (single-user mode) and users are added later, the owner workspace frozen in Arc never gets admin_prompt_enabled. Meanwhile WorkspacePool::build_workspace unconditionally calls .with_admin_prompt() for every tenant workspace.
Practical impact: an admin writes a system prompt then tests via CLI — they don't see it. All HTTP users do. This asymmetry is surprising.
Minimal fix: document that the owner workspace requires a server restart after the first user is created to activate admin_prompt_enabled. If full consistency is needed, move the check inside system_prompt_for_context_inner or make the flag reactive.
There was a problem hiding this comment.
Done — added a documentation comment block at the is_multi_tenant check explaining the startup-time evaluation behavior: the owner workspace requires a server restart after the first user is created to activate admin prompts. Tenant workspaces via WorkspacePool are unaffected since they always call .with_admin_prompt().
See commit 4efa4cb.
ilblackdragon
left a comment
There was a problem hiding this comment.
Verdict: Request changes
src/channels/web/handlers/system_prompt.rs— PR description claims a 64 KB size limit, but the handler only inherits the global 10 MB body limit. Either add the explicit cap or correct the description.src/workspace/mod.rs:1578— admin-prompt read usesif let Ok(doc) = …, silently swallowing DB outages as "no admin prompt". Log atdebug!; onlyDocumentNotFoundshould be silent.src/workspace/document.rs— unrelated rustdoc deletions onDocumentMetadata/HygieneMetadata/PatchResult. Should be reverted to keep the diff focused.- Cache: every turn pays an extra DB read for the admin doc — cache on
WorkspacePoolwith invalidation on PUT. - Tests: only
#[cfg(feature = "libsql")], contrary to the dual-backend rule. No handler-level tests for non-admin auth rejection, oversize body, or injection rejection. NoIf-Match/audit log on PUT (admin prompt changes affect every tenant).
Addresses PR review feedback: - Enforce 64 KB limit on system prompt content to prevent token budget exhaustion (the content is injected into every user's system prompt) - Add regression tests for the size limit (413 for oversized, not-413 for at-limit) - Document that is_multi_tenant is evaluated once at startup and the owner workspace requires a restart after the first user is created Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
@ilblackdragon Addressed the first two items in 4efa4cb: 64 KB size limit — Added an explicit Owner workspace asymmetry — Added a documentation comment block at the Still outstanding from your review:
|
- Restore rustdoc comments stripped from document.rs (DocumentMetadata, HygieneMetadata, DocumentVersion, VersionSummary, PatchResult, etc.) to keep the diff focused on feature additions only - Replace silent error swallowing (if let Ok) with discriminated match in admin prompt read — only DocumentNotFound is silent, other errors logged at debug! level - Cache admin system prompt on WorkspacePool to avoid an extra DB read on every turn; invalidated on PUT via invalidate_admin_prompt() - Add cache invalidation integration test Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Code ReviewWhat it does: Adds shared Real problems
Test gapsNo 401/403 tests for non-admin/unauth, no single-user 404 test, test #8 asserts Verdict: Approve with changes. The |
- is_reserved_scope: case-insensitive, whitespace-tolerant, and reserves the entire `__*__` namespace so future system scopes (alongside `__admin__`) cannot be impersonated by hand-crafted user IDs - admin system-prompt route: layer-level DefaultBodyLimit of 128 KB rejects oversized payloads before JSON parse, complementing the in-handler 64 KB content cap - system_prompt put_handler: clarify that the in-handler size check is a clearer-error fallback for the layer cap - users_create_handler: drop the dead is_reserved_scope check on a freshly-minted UUID; the guard belongs at a code path that actually accepts user-supplied IDs - expand is_reserved_scope tests for case, whitespace, and the wider `__*__` namespace Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
ilblackdragon
left a comment
There was a problem hiding this comment.
Approving — pushed the minor fixes directly: reserved-scope check is now case-insensitive and reserves the whole __*__ namespace, the admin system-prompt route gets a 128 KB DefaultBodyLimit layer to reject oversized bodies before JSON parse, and the dead UUID-collision check in user creation is removed. The two larger items from the review (startup-only is_multi_tenant gap for non-web channels, and admin-prompt cache invalidation only firing from the PUT handler) are not addressed here — leaving them as follow-ups since they need more design discussion.
* feat(workspace): admin system prompt shared with all users (nearai#2088) Introduce SYSTEM.md in a well-known __admin__ scope so admins can set a system prompt that all tenants receive. Gated behind multi-tenant mode (WorkspacePool sets admin_prompt_enabled on each workspace; owner workspace in app.rs also gets the flag when has_any_users() is true). New endpoints: - GET /api/admin/system-prompt — read admin system prompt - PUT /api/admin/system-prompt — set admin system prompt (64 KB limit) Safety: - SYSTEM.md added to injection scan list - is_reserved_scope() guard on user creation (defense-in-depth) - Multi-tenancy gate on both API and prompt assembly layers Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * chore: remove review audit file from tracked files Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: add 64 KB size limit to admin system prompt PUT handler Addresses PR review feedback: - Enforce 64 KB limit on system prompt content to prevent token budget exhaustion (the content is injected into every user's system prompt) - Add regression tests for the size limit (413 for oversized, not-413 for at-limit) - Document that is_multi_tenant is evaluated once at startup and the owner workspace requires a restart after the first user is created Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: address remaining review feedback on admin system prompt - Restore rustdoc comments stripped from document.rs (DocumentMetadata, HygieneMetadata, DocumentVersion, VersionSummary, PatchResult, etc.) to keep the diff focused on feature additions only - Replace silent error swallowing (if let Ok) with discriminated match in admin prompt read — only DocumentNotFound is silent, other errors logged at debug! level - Cache admin system prompt on WorkspacePool to avoid an extra DB read on every turn; invalidated on PUT via invalidate_admin_prompt() - Add cache invalidation integration test Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(workspace): tighten reserved-scope check and admin-prompt body limit - is_reserved_scope: case-insensitive, whitespace-tolerant, and reserves the entire `__*__` namespace so future system scopes (alongside `__admin__`) cannot be impersonated by hand-crafted user IDs - admin system-prompt route: layer-level DefaultBodyLimit of 128 KB rejects oversized payloads before JSON parse, complementing the in-handler 64 KB content cap - system_prompt put_handler: clarify that the in-handler size check is a clearer-error fallback for the layer cap - users_create_handler: drop the dead is_reserved_scope check on a freshly-minted UUID; the guard belongs at a code path that actually accepts user-supplied IDs - expand is_reserved_scope tests for case, whitespace, and the wider `__*__` namespace Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: Illia Polosukhin <ilblackdragon@gmail.com>
Summary
SYSTEM.mdin a well-known__admin__scope so admins can set a system prompt that all tenants receiveGET/PUT /api/admin/system-promptwith 64 KB size limitCloses #2088
How it works
SYSTEM.mdto__admin__scope viaPUT /api/admin/system-promptsystem_prompt_for_context_inner(), whenadmin_prompt_enabledis true, readsSYSTEM.mddirectly from__admin__scopeMulti-tenancy gating
workspace_pool.is_some()→ 404 in single-user modeadmin_prompt_enabledflag on Workspace — set by WorkspacePool and in app.rs whenhas_any_users()is trueFiles changed
src/workspace/document.rspaths::SYSTEM,ADMIN_SCOPE,is_reserved_scope()src/workspace/mod.rsadmin_prompt_enabledfield/builder, gated read in prompt assemblysrc/app.rsadmin_prompt_enabledon owner workspace in multi-tenant modesrc/channels/web/server.rs.with_admin_prompt()in WorkspacePool, route registrationsrc/channels/web/handlers/system_prompt.rssrc/channels/web/handlers/users.rssrc/channels/web/types.rstests/admin_system_prompt.rsTest plan
cargo clippy --all --all-features— zero warningscargo test --lib— 4289 passedcargo test --test admin_system_prompt --features libsql— 6 passed🤖 Generated with Claude Code