diff --git a/TELEMETRY.md b/TELEMETRY.md index aebc2232b9..59ff621ed6 100644 --- a/TELEMETRY.md +++ b/TELEMETRY.md @@ -33,6 +33,7 @@ use the query recipes below to find the failing turn and next query. | `messaging.destination.name` | Slack channel | logs, spans | channel-scoped search | | `gen_ai.tool.name` | tool name | tool spans/logs | tool failures | | `app.credential.provider` | auth provider | auth logs | auth/resume search | +| `user.id` | accountable person | logs, spans, errors | person's actions | | `app.task.id` | scheduled/event automation id | task lifecycle logs | task timeline | | `app.task.run.id` | scheduled-automation run id | scheduler run logs | run outcome | | `app.dispatch.id` | agent dispatch id | task/dispatch logs | fire conversation | @@ -127,6 +128,16 @@ fields=timestamp,event.name,app.task.id,app.task.run.id,app.dispatch.id,app.task sort=-timestamp ``` +Credentialed provider writes, with whose authority they used. Write-grant +egress requests are audit events (`app.log.audit:true`), so Sentry keeps them +in production. Other info events stay out of Sentry in production. + +```text +dataset=logs query='app.log.audit:true event.name:sandbox.egress.upstream.requested' +fields=timestamp,user.id,app.actor.type,app.actor.id,app.credential.allowed_when,app.task.id,gen_ai.conversation.id,app.provider.name,app.grant.name,http.request.method,server.address,url.path,http.response.status_code +sort=-timestamp +``` + Auth, credential, and resume failures. ```text @@ -135,6 +146,23 @@ fields=timestamp,event.name,gen_ai.conversation.id,app.credential.provider,app.a sort=-timestamp ``` +## Identity + +Runs, resumes, and the sandbox egress proxy bind the same identity attributes. +Nested logs and spans inherit them. + +- `user.id`, `user.name`, `user.email`, `user.full_name`: the accountable + person. For a user Actor, this is that user. For a system Actor, this is the + owner of delegated credentials, such as the Automation creator. It is empty + when a system run has no delegated credentials. Sentry also uses `user.id` + for its user features. +- `app.actor.type`, `app.actor.id`: who acted. The type is `user` or `system`. +- `app.credential.allowed_when`, `app.task.id`: why delegated credentials were + allowed, and for which Automation. + +Do not add app-specific keys under `user.*`. Put facts about the request in +`app.*`. + ## Domains ### Database Queries @@ -259,6 +287,12 @@ Attributes: `gen_ai.tool.name`, `gen_ai.tool.call.id`, `app.sandbox.search.result_bytes`, `app.sandbox.search.limit`, `app.sandbox.search.limit_reached`, `app.tool.missing_annotations` +Sandbox egress events: `sandbox.egress.upstream.requested` is logged for every +forwarded request outside production. In production, it is logged only for +write grants, as an audit event. Attributes: `app.provider.name`, +`app.grant.name`, `app.grant.access`, `app.log.audit`, and the identity +attributes below. The proxy binds them from the signed credential token. + ### Scheduled automations A scheduled automation was created, claimed, dispatched, completed, or failed diff --git a/packages/junior/src/chat/agent/index.ts b/packages/junior/src/chat/agent/index.ts index 0ccf882752..3114321617 100644 --- a/packages/junior/src/chat/agent/index.ts +++ b/packages/junior/src/chat/agent/index.ts @@ -58,7 +58,10 @@ import { type ConversationMessageProvenance, } from "@/chat/conversations/provenance"; import type { Actor } from "@/chat/actor"; -import { credentialUserSubjectId } from "@/chat/credentials/context"; +import { + actorLogContext, + credentialUserSubjectId, +} from "@/chat/credentials/context"; import { GEN_AI_PROVIDER_NAME, completeObject, @@ -240,9 +243,7 @@ export async function executeAgentRun( const conversationPrivacy = resolveConversationPrivacy({ visibility, }); - const credentialActor = run.credentialContext?.actor; const actor = actorFromRun(run); - const userActor = actor && "userId" in actor ? actor : undefined; const runLogContext: LogContext = { conversationId: run.conversationId, platform: @@ -264,20 +265,8 @@ export async function executeAgentRun( : run.source.kind === "web" || run.source.kind === "local" ? run.source.conversationId : run.conversationId), - userId: userActor?.userId, - userName: userActor?.userName, - userEmail: userActor?.email, + ...actorLogContext(actor, run.credentialContext), runId: run.runId, - actorType: credentialActor - ? "type" in credentialActor - ? credentialActor.type - : "system" - : undefined, - actorId: credentialActor - ? "type" in credentialActor - ? credentialActor.userId - : credentialActor.name - : undefined, assistantUserName: botConfig.userName, }; return withLogContext(runLogContext, () => diff --git a/packages/junior/src/chat/agent/sandbox.ts b/packages/junior/src/chat/agent/sandbox.ts index 9dc38f233f..1c3bdf207c 100644 --- a/packages/junior/src/chat/agent/sandbox.ts +++ b/packages/junior/src/chat/agent/sandbox.ts @@ -35,6 +35,7 @@ export interface AgentSandboxOptions { traceContext: LogContext; tracePropagation?: SandboxEgressTracePropagationConfig; egressSignals?: SandboxEgressSignalTransport; + conversationId?: string; credentialEgress?: CredentialContext; actor?: Actor; locationConfiguration?: LocationConfigurationService; @@ -166,6 +167,7 @@ export function createAgentSandbox(options: AgentSandboxOptions): AgentSandbox { traceContext: options.traceContext, tracePropagation: options.tracePropagation, egressSignals: options.egressSignals, + conversationId: options.conversationId, credentialEgress: options.credentialEgress, prepare: options.prepareSandbox, prepareWorkspace: options.prepareWorkspace, diff --git a/packages/junior/src/chat/agent/tools.ts b/packages/junior/src/chat/agent/tools.ts index 2623744032..5324e1dcec 100644 --- a/packages/junior/src/chat/agent/tools.ts +++ b/packages/junior/src/chat/agent/tools.ts @@ -266,6 +266,7 @@ export async function wireAgentTools( traceContext: args.spanContext, tracePropagation: args.run.environment?.sandboxTracePropagation, egressSignals: args.run.environment?.sandboxEgressSignals, + conversationId: args.run.conversationId, credentialEgress: args.run.credentialContext, actor: args.currentActor, locationConfiguration: args.run.environment?.locationConfiguration, diff --git a/packages/junior/src/chat/credentials/context.ts b/packages/junior/src/chat/credentials/context.ts index e31d73745c..ff77d4aac1 100644 --- a/packages/junior/src/chat/credentials/context.ts +++ b/packages/junior/src/chat/credentials/context.ts @@ -1,5 +1,6 @@ import { z } from "zod"; -import { parseActorUserId, type Actor } from "@/chat/actor"; +import { isUserActor, parseActorUserId, type Actor } from "@/chat/actor"; +import type { LogContext } from "@/chat/log-context"; const exactActorIdSchema = z .string() @@ -159,6 +160,59 @@ export function credentialContextForActor( return { actor: { type: "user", userId: actor.userId } }; } +/** + * Name the person accountable for credential authority in logs. + * + * `userId` is the user actor, or the owner of delegated credentials when the + * system acts for an Automation or Watch. `actorType` says who acted. + */ +export function credentialLogContext(context: CredentialContext): LogContext { + if ("type" in context.actor) { + return { + userId: context.actor.userId, + actorType: "user", + actorId: context.actor.userId, + }; + } + const subject = "subject" in context ? context.subject : undefined; + return { + actorType: "system", + actorId: context.actor.name, + ...(subject + ? { + userId: subject.userId, + credentialAllowedWhen: subject.allowedWhen, + ...("taskId" in subject ? { taskId: subject.taskId } : undefined), + } + : undefined), + }; +} + +/** + * Name the run's people in logs: the accountable person and who acted. + * + * A user actor is the accountable person. A system actor names the owner of + * delegated credentials, when the run has them. + */ +export function actorLogContext( + actor: Actor | undefined, + credentialContext: CredentialContext | undefined, +): LogContext { + const credential = credentialContext + ? credentialLogContext(credentialContext) + : {}; + if (!isUserActor(actor)) { + return credential; + } + return { + ...credential, + userId: actor.userId, + userName: actor.userName, + userEmail: actor.email, + userFullName: actor.fullName, + }; +} + /** Return the user whose OAuth token may satisfy this credential request. */ export function credentialUserSubjectId( context: CredentialContext, diff --git a/packages/junior/src/chat/egress/credentialed.ts b/packages/junior/src/chat/egress/credentialed.ts index 92558eb861..670e62ccdd 100644 --- a/packages/junior/src/chat/egress/credentialed.ts +++ b/packages/junior/src/chat/egress/credentialed.ts @@ -1,4 +1,4 @@ -import { logException, logInfo, logWarn } from "@/chat/logging"; +import { logAudit, logException, logInfo, logWarn } from "@/chat/logging"; import { onPluginEgressResponse } from "@/chat/plugins/credential-hooks"; import { matchesSandboxEgressDomain } from "@/chat/sandbox/egress/policy"; import { @@ -170,15 +170,23 @@ function egressAttributes(input: { status?: number; }): Record { return { - ...(input.egressId ? { "app.sandbox.egress_id": input.egressId } : undefined), + ...(input.egressId + ? { "app.sandbox.egress_id": input.egressId } + : undefined), ...(input.provider ? { "app.provider.name": input.provider } : undefined), ...(input.grantName ? { "app.grant.name": input.grantName } : undefined), - ...(input.grantAccess ? { "app.grant.access": input.grantAccess } : undefined), - ...(input.grantReason ? { "app.grant.reason": input.grantReason } : undefined), + ...(input.grantAccess + ? { "app.grant.access": input.grantAccess } + : undefined), + ...(input.grantReason + ? { "app.grant.reason": input.grantReason } + : undefined), ...(input.host ? { "server.address": input.host } : undefined), ...(input.method ? { "http.request.method": input.method } : undefined), ...(input.path ? { "url.path": input.path } : undefined), - ...(input.status ? { "http.response.status_code": input.status } : undefined), + ...(input.status + ? { "http.response.status_code": input.status } + : undefined), }; } @@ -270,6 +278,11 @@ function isEgressPolicyDenied(error: unknown): error is EgressPolicyDenied { ); } +/** + * Record one forwarded provider request. Write-grant requests are audit + * events, so they reach Sentry in production. Other requests log only outside + * production. + */ function logSandboxEgressUpstreamRequest(input: { egressId: string; grantAccess?: "read" | "write"; @@ -280,11 +293,12 @@ function logSandboxEgressUpstreamRequest(input: { upstream: Response; upstreamUrl: URL; }): void { - if (!shouldLogSandboxEgressInfo()) { + const isWrite = input.grantAccess === "write"; + if (!isWrite && !shouldLogSandboxEgressInfo()) { return; } - logInfo("sandbox.egress.upstream.requested", { + (isWrite ? logAudit : logInfo)("sandbox.egress.upstream.requested", { ...egressAttributes({ egressId: input.egressId, grantAccess: input.grantAccess, @@ -563,7 +577,9 @@ async function recordSandboxAuthRequired(input: { provider: input.provider, grant: input.grant, kind: input.kind ?? "auth_required", - ...(input.authorization ? { authorization: input.authorization } : undefined), + ...(input.authorization + ? { authorization: input.authorization } + : undefined), message: input.message, }); } @@ -687,7 +703,9 @@ export async function executeCredentialedEgressRequest(input: { const recordPermissionDenied = deps.recordPermissionDenied ?? recordSandboxPermissionDenied; - async function resolveLease(): Promise { + async function resolveLease(): Promise< + SandboxEgressCredentialLease | Response + > { try { return await issueCredentialLease( provider, @@ -792,8 +810,7 @@ export async function executeCredentialedEgressRequest(input: { return intercepted; } - const requestBody = - body instanceof ArrayBuffer ? body.slice(0) : body; + const requestBody = body instanceof ArrayBuffer ? body.slice(0) : body; const upstream = await fetchImpl(upstreamUrl, { method: request.method, headers, diff --git a/packages/junior/src/chat/log-context.ts b/packages/junior/src/chat/log-context.ts index c8be01e2aa..7ddad955bd 100644 --- a/packages/junior/src/chat/log-context.ts +++ b/packages/junior/src/chat/log-context.ts @@ -1,4 +1,5 @@ import { AsyncLocalStorage } from "node:async_hooks"; +import { normalizeIdentityEmail } from "@/chat/identities/identity"; export type LogAttributeValue = string | number | boolean | string[]; export type LogAttributes = Record; @@ -10,12 +11,18 @@ export interface LogContext { requestId?: string; messageConversationId?: string; destinationName?: string; + /** The accountable person: the user Actor, or the delegated credential owner. */ userId?: string; userName?: string; userEmail?: string; + userFullName?: string; runId?: string; + /** Who acted: `user` or `system`. */ actorType?: string; actorId?: string; + /** Why delegated credentials were allowed, and for which Automation. */ + taskId?: string; + credentialAllowedWhen?: string; assistantUserName?: string; modelId?: string; skillName?: string; @@ -57,11 +64,15 @@ export function logContextToAttributes(context: LogContext): LogAttributes { context.platform === "slack" ? "slack" : context.platform, "messaging.message.conversation_id": context.messageConversationId, "messaging.destination.name": context.destinationName, - "enduser.id": context.userId, - "enduser.pseudo.id": context.userName, + "user.id": context.userId, + "user.name": context.userName, + "user.email": normalizeIdentityEmail(context.userEmail), + "user.full_name": context.userFullName, "app.run.id": context.runId, "app.actor.type": context.actorType, "app.actor.id": context.actorId, + "app.task.id": context.taskId, + "app.credential.allowed_when": context.credentialAllowedWhen, "gen_ai.agent.name": context.assistantUserName, "gen_ai.request.model": context.modelId, "app.skill.name": context.skillName, diff --git a/packages/junior/src/chat/logging.ts b/packages/junior/src/chat/logging.ts index b497f66fcb..4bd3eedb47 100644 --- a/packages/junior/src/chat/logging.ts +++ b/packages/junior/src/chat/logging.ts @@ -180,8 +180,10 @@ const CONSOLE_PRIORITY_INDEX: Map = new Map( const CONSOLE_ALWAYS_HIDDEN_KEYS = new Set([ "gen_ai.agent.name", "app.platform", - "enduser.id", - "enduser.pseudo.id", + "user.id", + "user.name", + "user.email", + "user.full_name", "http.request.method", "messaging.system", "url.full", @@ -217,8 +219,18 @@ function getSentryEnvironment(): string { .toLowerCase(); } -function shouldSuppressInfoLog(level: LogLevel): boolean { - return getSentryEnvironment() === "production" && level === "info"; +/** Marks an audit event. Sentry keeps it in production. */ +const AUDIT_LOG_ATTRIBUTE = "app.log.audit"; + +function shouldSuppressInfoLog( + level: LogLevel, + attributes: LogAttributes, +): boolean { + return ( + getSentryEnvironment() === "production" && + level === "info" && + attributes[AUDIT_LOG_ATTRIBUTE] !== true + ); } function shouldEmitConsole(level: LogLevel): boolean { @@ -643,7 +655,7 @@ function emitSentry( body: string, attributes: LogAttributes, ): void { - if (shouldSuppressInfoLog(level)) { + if (shouldSuppressInfoLog(level, attributes)) { return; } @@ -1541,6 +1553,20 @@ export function logInfo( log.info(eventName, attributes); } +/** + * Emit an audit event: an info event that Sentry also keeps in production. + * + * Use it only to record who caused an external side effect and why. Other + * info events stay out of Sentry in production. Sentry log retention is not + * durable audit storage. + */ +export function logAudit( + eventName: string, + attributes: Record = {}, +): void { + log.info(eventName, { ...attributes, [AUDIT_LOG_ATTRIBUTE]: true }); +} + /** * Emit a warning event with occurrence-specific attributes. * diff --git a/packages/junior/src/chat/providers/slack/resume.ts b/packages/junior/src/chat/providers/slack/resume.ts index 7550724012..3acdd15b1e 100644 --- a/packages/junior/src/chat/providers/slack/resume.ts +++ b/packages/junior/src/chat/providers/slack/resume.ts @@ -55,6 +55,7 @@ import { } from "@/chat/slack/assistant-thread/status"; import { sendSlackReply } from "@/chat/slack/reply"; import { isUserActor, type Actor } from "@/chat/actor"; +import { actorLogContext } from "@/chat/credentials/context"; import { postSlackMessage as postSlackApiMessage } from "@/chat/slack/outbound"; import { startProcessingReactionForMessage, @@ -196,12 +197,10 @@ interface ResumePreparedTurn { } function getResumeLogContext(args: ResumeSlackTurnArgs): LogContext { - const actor = args.run?.actor; return { conversationId: args.conversationId, messageConversationId: args.conversationId, - userId: isUserActor(actor) ? actor.userId : undefined, - userName: isUserActor(actor) ? actor.userName : undefined, + ...actorLogContext(args.run?.actor, args.run?.credentialContext), destinationName: args.channelId, assistantUserName: botConfig.userName, modelId: defaultModelId(botConfig), diff --git a/packages/junior/src/chat/sandbox/egress/proxy.ts b/packages/junior/src/chat/sandbox/egress/proxy.ts index 6f61d37782..fd321e44b6 100644 --- a/packages/junior/src/chat/sandbox/egress/proxy.ts +++ b/packages/junior/src/chat/sandbox/egress/proxy.ts @@ -1,4 +1,6 @@ -import { logWarn, withSpan } from "@/chat/logging"; +import { credentialLogContext } from "@/chat/credentials/context"; +import type { LogContext } from "@/chat/log-context"; +import { logWarn, setSentryUser, withSpan } from "@/chat/logging"; import { executeCredentialedEgressRequest, type CredentialedEgressHttpInterceptor, @@ -8,6 +10,7 @@ import { verifyVercelSandboxOidcToken } from "@/chat/sandbox/egress/oidc"; import { parseSandboxEgressCredentialToken, SANDBOX_EGRESS_PROXY_PATH, + type SandboxEgressCredentialContext, } from "@/chat/sandbox/egress/session"; import { shouldPropagateSandboxEgressTrace, @@ -65,12 +68,16 @@ function egressAttributes(input: { status?: number; }): Record { return { - ...(input.egressId ? { "app.sandbox.egress_id": input.egressId } : undefined), + ...(input.egressId + ? { "app.sandbox.egress_id": input.egressId } + : undefined), ...(input.provider ? { "app.provider.name": input.provider } : undefined), ...(input.host ? { "server.address": input.host } : undefined), ...(input.method ? { "http.request.method": input.method } : undefined), ...(input.path ? { "url.path": input.path } : undefined), - ...(input.status ? { "http.response.status_code": input.status } : undefined), + ...(input.status + ? { "http.response.status_code": input.status } + : undefined), }; } @@ -223,20 +230,49 @@ function buildUpstreamUrl(request: Request): UpstreamUrlResult { } } +/** + * Name the Conversation and the accountable person from the verified token. + * Each proxy request is a new HTTP request with no run log context, so the + * proxy binds the same fields that the run binds. + */ +function sandboxEgressLogContext( + context: SandboxEgressCredentialContext, +): LogContext { + return { + ...(context.conversationId + ? { conversationId: context.conversationId } + : undefined), + ...credentialLogContext(context.credentials), + }; +} + /** Continue inbound trace context only after sandbox egress verification succeeds. */ function continueSandboxEgressTrace( request: Request, upstreamHost: string, tracePropagation: SandboxEgressTracePropagationConfig, + credentialContext: SandboxEgressCredentialContext, callback: () => Promise, ): Promise { const sentryTrace = request.headers.get("sentry-trace") ?? undefined; const baggage = request.headers.get("baggage") ?? undefined; + const logContext = sandboxEgressLogContext(credentialContext); const run = () => - withSpan("sandbox.egress", "http.server", {}, callback, { - "http.request.method": request.method, - "url.path": redactedProxyPath(new URL(request.url).pathname), - }); + withSpan( + "sandbox.egress", + "http.server", + logContext, + async () => { + setSentryUser( + logContext.userId ? { id: logContext.userId } : undefined, + ); + return await callback(); + }, + { + "http.request.method": request.method, + "url.path": redactedProxyPath(new URL(request.url).pathname), + }, + ); if ( !shouldPropagateSandboxEgressTrace(upstreamHost, tracePropagation) || (!sentryTrace && !baggage) @@ -361,6 +397,7 @@ export async function proxySandboxEgressRequest( request, upstreamUrl.hostname, deps.tracePropagation ?? {}, + credentialContext, async () => await executeCredentialedEgressRequest({ activeEgressId, diff --git a/packages/junior/src/chat/sandbox/egress/schemas.ts b/packages/junior/src/chat/sandbox/egress/schemas.ts index 3c9f829c35..65744e5208 100644 --- a/packages/junior/src/chat/sandbox/egress/schemas.ts +++ b/packages/junior/src/chat/sandbox/egress/schemas.ts @@ -17,6 +17,8 @@ export const sandboxEgressGrantSchema = pluginGrantSchema; export const sandboxEgressCredentialContextSchema = z .object({ credentials: credentialContextSchema, + /** Conversation that the credentials act for. Telemetry only; it grants nothing. */ + conversationId: z.string().min(1).optional(), egressId: z.string().min(1), expiresAtMs: finiteNumberSchema, contextId: z.string().min(1), diff --git a/packages/junior/src/chat/sandbox/egress/session.ts b/packages/junior/src/chat/sandbox/egress/session.ts index 6d3bc97bb7..9ec867a4d9 100644 --- a/packages/junior/src/chat/sandbox/egress/session.ts +++ b/packages/junior/src/chat/sandbox/egress/session.ts @@ -156,6 +156,7 @@ function getSandboxEgressSecret(): string { * which Junior actor and credential context that VM is allowed to use. */ export function createSandboxEgressCredentialToken(input: { + conversationId?: string; credentials: CredentialContext; egressId: string; ttlMs?: number; @@ -163,6 +164,9 @@ export function createSandboxEgressCredentialToken(input: { const ttlMs = Math.max(1, input.ttlMs ?? DEFAULT_SESSION_TTL_MS); const now = Date.now(); const context: SandboxEgressCredentialContext = { + ...(input.conversationId + ? { conversationId: input.conversationId } + : undefined), credentials: input.credentials, egressId: input.egressId, expiresAtMs: now + ttlMs, diff --git a/packages/junior/src/chat/sandbox/sandbox.ts b/packages/junior/src/chat/sandbox/sandbox.ts index 3f7c832d81..45cf4d42be 100644 --- a/packages/junior/src/chat/sandbox/sandbox.ts +++ b/packages/junior/src/chat/sandbox/sandbox.ts @@ -103,6 +103,8 @@ export interface SandboxOptions { timeoutMs?: number; traceContext?: LogContext; tracePropagation?: SandboxEgressTracePropagationConfig; + /** Conversation that sandbox egress records for its credentialed requests. */ + conversationId?: string; credentialEgress?: CredentialContext; egressSignals?: SandboxEgressSignalTransport; prepare?: (workspace: SandboxWorkspace) => void | Promise; @@ -192,6 +194,7 @@ export function createSandbox(options: SandboxOptions): SandboxAccess { } const now = Date.now(); const token = createSandboxEgressCredentialToken({ + conversationId: options.conversationId, credentials: credentialEgress, egressId, ttlMs: sandboxEgressTokenTtlMs, @@ -431,7 +434,9 @@ export function createSandbox(options: SandboxOptions): SandboxAccess { stdout_truncated: result.stdoutTruncated, stderr_truncated: result.stderrTruncated, ...(authRequired ? { auth_required: authRequired } : undefined), - ...(permissionDenied ? { permission_denied: permissionDenied } : undefined), + ...(permissionDenied + ? { permission_denied: permissionDenied } + : undefined), }) as T; }; @@ -633,8 +638,12 @@ export function createSandbox(options: SandboxOptions): SandboxAccess { context.setToolCallSpanAttributes, ), pattern, - ...(typeof rawInput.path === "string" ? { path: rawInput.path } : undefined), - ...(typeof rawInput.glob === "string" ? { glob: rawInput.glob } : undefined), + ...(typeof rawInput.path === "string" + ? { path: rawInput.path } + : undefined), + ...(typeof rawInput.glob === "string" + ? { glob: rawInput.glob } + : undefined), ...(typeof rawInput.ignoreCase === "boolean" ? { ignoreCase: rawInput.ignoreCase } : undefined), @@ -681,7 +690,9 @@ export function createSandbox(options: SandboxOptions): SandboxAccess { context.setToolCallSpanAttributes, ), pattern, - ...(typeof rawInput.path === "string" ? { path: rawInput.path } : undefined), + ...(typeof rawInput.path === "string" + ? { path: rawInput.path } + : undefined), ...(limit ? { limit } : undefined), }); setSpanStatus("ok"); @@ -706,7 +717,9 @@ export function createSandbox(options: SandboxOptions): SandboxAccess { async () => { const response = await listDir({ fs: fileSystem, - ...(typeof rawInput.path === "string" ? { path: rawInput.path } : undefined), + ...(typeof rawInput.path === "string" + ? { path: rawInput.path } + : undefined), ...(limit ? { limit } : undefined), }); setSpanStatus("ok"); @@ -792,9 +805,12 @@ export function createSandbox(options: SandboxOptions): SandboxAccess { } catch (error) { const notReady = getWorkspaceSnapshotNotReadyError(error); if (notReady) { - throw new ToolInputError(workspaceSnapshotNotReadyUserMessage(notReady), { - cause: notReady, - }); + throw new ToolInputError( + workspaceSnapshotNotReadyUserMessage(notReady), + { + cause: notReady, + }, + ); } if (isSandboxUnavailableError(error)) { throw createSandboxUnavailableToolError(operation, error); diff --git a/packages/junior/src/chat/slack/user.ts b/packages/junior/src/chat/slack/user.ts index c97ab1ce59..7cf546deee 100644 --- a/packages/junior/src/chat/slack/user.ts +++ b/packages/junior/src/chat/slack/user.ts @@ -90,7 +90,7 @@ export async function lookupSlackUser( if (!response.ok) { logWarn("slack.user.lookup.failed", { - "enduser.id": userId, + "app.slack.user_id": userId, "app.slack.team_id": teamId, "http.response.status_code": response.status, }); @@ -115,7 +115,7 @@ export async function lookupSlackUser( return result; } catch (error) { logWarn("slack.user.lookup.failed", { - "enduser.id": userId, + "app.slack.user_id": userId, "app.slack.team_id": teamId, "exception.message": error instanceof Error ? error.message : String(error), diff --git a/packages/junior/tests/component/handlers/sandbox-egress-proxy.test.ts b/packages/junior/tests/component/handlers/sandbox-egress-proxy.test.ts index 283e3f1f93..7413c959c4 100644 --- a/packages/junior/tests/component/handlers/sandbox-egress-proxy.test.ts +++ b/packages/junior/tests/component/handlers/sandbox-egress-proxy.test.ts @@ -60,6 +60,7 @@ vi.mock("@/chat/sentry", () => ({ continueTrace: continueTraceMock, getActiveSpan: () => undefined, logger: loggerMock, + setUser: vi.fn(), spanToJSON: () => ({}), startSpan: startSpanMock, })); diff --git a/packages/junior/tests/component/runtime/agent-run-error-path.test.ts b/packages/junior/tests/component/runtime/agent-run-error-path.test.ts index 657b001d68..1a33c3ff08 100644 --- a/packages/junior/tests/component/runtime/agent-run-error-path.test.ts +++ b/packages/junior/tests/component/runtime/agent-run-error-path.test.ts @@ -89,8 +89,8 @@ describe("executeAgentRun error path", () => { expect(failure?.attributes).toMatchObject({ "app.platform": "local", "app.run.id": "run-context-failure", - "enduser.id": "local-user", - "enduser.pseudo.id": "alice", + "user.id": "local-user", + "user.name": "alice", "gen_ai.conversation.id": LOCAL_DESTINATION.conversationId, "messaging.destination.name": LOCAL_DESTINATION.conversationId, "messaging.message.conversation_id": LOCAL_DESTINATION.conversationId, diff --git a/packages/junior/tests/component/slack/inbound-telemetry.test.ts b/packages/junior/tests/component/slack/inbound-telemetry.test.ts index 3a92f8977d..30ed3ba291 100644 --- a/packages/junior/tests/component/slack/inbound-telemetry.test.ts +++ b/packages/junior/tests/component/slack/inbound-telemetry.test.ts @@ -64,7 +64,7 @@ describe("Slack inbound telemetry", () => { name: "slack.message.ingress", op: "slack.message.ingress", attributes: expect.objectContaining({ - "enduser.id": "U123", + "user.id": "U123", "app.slack.user_id": "U123", "app.slack.team_id": "T123", "app.slack.user_team": "TEXTERNAL", diff --git a/packages/junior/tests/integration/sandbox-egress-proxy.test.ts b/packages/junior/tests/integration/sandbox-egress-proxy.test.ts index de0f8eb9aa..eb421325c6 100644 --- a/packages/junior/tests/integration/sandbox-egress-proxy.test.ts +++ b/packages/junior/tests/integration/sandbox-egress-proxy.test.ts @@ -589,7 +589,12 @@ describe("sandbox egress proxy integration", () => { }); it("reuses broker-managed credentials across read and write egress", async () => { + const records: EmittedLogRecord[] = []; + const unregister = modules.logging.registerLogRecordSink((record) => { + records.push(record); + }); const credentialToken = modules.session.createSandboxEgressCredentialToken({ + conversationId: "conversation-1", credentials: { actor: { type: "user", userId: ACTOR_ID } }, egressId: EGRESS_ID, ttlMs: 60_000, @@ -628,11 +633,24 @@ describe("sandbox egress proxy integration", () => { }, ); + unregister(); + expect(writeResponse.status).toBe(200); await expect(writeResponse.text()).resolves.toBe( "Bearer integration-egress-token", ); expect(upstreamFetch).toHaveBeenCalledTimes(2); + const writeAudit = records.find( + (record) => + record.eventName === "sandbox.egress.upstream.requested" && + record.attributes["http.request.method"] === "POST", + ); + expect(writeAudit?.attributes).toMatchObject({ + "app.actor.type": "user", + "app.log.audit": true, + "gen_ai.conversation.id": "conversation-1", + "user.id": ACTOR_ID, + }); }); it("intercepts credential-injected provider traffic before live forwarding", async () => { diff --git a/packages/junior/tests/unit/credentials/credential-context.test.ts b/packages/junior/tests/unit/credentials/credential-context.test.ts index 314bf4bd89..1f8da39a6a 100644 --- a/packages/junior/tests/unit/credentials/credential-context.test.ts +++ b/packages/junior/tests/unit/credentials/credential-context.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it } from "vitest"; import { + credentialLogContext, credentialUserSubjectId, parseCredentialContext, } from "@/chat/credentials/context"; @@ -41,6 +42,35 @@ describe("credential context", () => { ).toBeUndefined(); }); + it("logs the delegated owner as the accountable user of a system run", () => { + expect( + credentialLogContext({ actor: { type: "user", userId: "U123" } }), + ).toEqual({ userId: "U123", actorType: "user", actorId: "U123" }); + expect( + credentialLogContext({ + actor: { platform: "system", name: "scheduler" }, + subject: { + type: "user", + userId: "U123", + allowedWhen: "scheduled-automation", + taskId: "task-1", + binding: { + type: "scheduled-automation", + plugin: "scheduler", + taskId: "task-1", + signature: "v1=test", + }, + }, + }), + ).toEqual({ + userId: "U123", + actorType: "system", + actorId: "scheduler", + taskId: "task-1", + credentialAllowedWhen: "scheduled-automation", + }); + }); + it("parses untrusted egress contexts with the same actor rules", () => { expect( parseCredentialContext({ diff --git a/packages/junior/tests/unit/logging/sentry-context.test.ts b/packages/junior/tests/unit/logging/sentry-context.test.ts index 021b4f77ca..8f6ad1f19b 100644 --- a/packages/junior/tests/unit/logging/sentry-context.test.ts +++ b/packages/junior/tests/unit/logging/sentry-context.test.ts @@ -105,11 +105,8 @@ describe("Sentry context", () => { "messaging.destination.name", "C123", ); - expect(sentry.setTag).not.toHaveBeenCalledWith("enduser.id", "U123"); - expect(sentry.setTag).not.toHaveBeenCalledWith( - "enduser.pseudo.id", - "alice", - ); + expect(sentry.setTag).not.toHaveBeenCalledWith("user.id", "U123"); + expect(sentry.setTag).not.toHaveBeenCalledWith("user.name", "alice"); expect(sentry.setTag).not.toHaveBeenCalledWith("app.run.id", "run_123"); }); @@ -147,13 +144,14 @@ describe("Sentry context", () => { "gen_ai.conversation.id", "thread_123", ); - expect(scope.setTag).not.toHaveBeenCalledWith("enduser.id", "U123"); - expect(scope.setTag).not.toHaveBeenCalledWith("enduser.pseudo.id", "alice"); + expect(scope.setTag).not.toHaveBeenCalledWith("user.id", "U123"); + expect(scope.setTag).not.toHaveBeenCalledWith("user.name", "alice"); expect(scope.setContext).toHaveBeenCalledWith( "app", expect.objectContaining({ - "enduser.id": "U123", - "enduser.pseudo.id": "alice", + "user.id": "U123", + "user.name": "alice", + "user.email": "alice@example.com", }), ); }); diff --git a/packages/junior/tests/unit/logging/structured-event.test.ts b/packages/junior/tests/unit/logging/structured-event.test.ts index ca16a13cea..1c026bc169 100644 --- a/packages/junior/tests/unit/logging/structured-event.test.ts +++ b/packages/junior/tests/unit/logging/structured-event.test.ts @@ -1,13 +1,18 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import type { EmittedLogRecord } from "@/chat/logging"; -async function loadLoggingModule() { +/** Sentry logger methods that a test observes. */ +interface SentryLoggerStub { + info?: (body: string, attributes: Record) => void; +} + +async function loadLoggingModule(sentryLogger: SentryLoggerStub = {}) { vi.resetModules(); vi.doMock("@/chat/sentry", () => ({ captureException: () => undefined, captureMessage: () => undefined, getActiveSpan: () => ({ sampled: true }), - logger: {}, + logger: sentryLogger, setTag: () => undefined, setUser: () => undefined, spanToJSON: () => ({ @@ -21,6 +26,7 @@ async function loadLoggingModule() { } afterEach(() => { + vi.unstubAllEnvs(); vi.restoreAllMocks(); vi.resetModules(); vi.doUnmock("@/chat/sentry"); @@ -67,6 +73,30 @@ describe("structured log events", () => { ]); }); + it("sends audit events to Sentry in production but not other info events", async () => { + vi.stubEnv("SENTRY_ENVIRONMENT", "production"); + vi.spyOn(console, "info").mockImplementation(() => undefined); + vi.spyOn(console, "log").mockImplementation(() => undefined); + const info = vi.fn(); + const { logAudit, logInfo } = await loadLoggingModule({ info }); + + logInfo("sandbox.egress.upstream.requested", { + "app.grant.access": "read", + }); + logAudit("sandbox.egress.upstream.requested", { + "app.grant.access": "write", + }); + + expect(info).toHaveBeenCalledTimes(1); + expect(info).toHaveBeenCalledWith( + "sandbox.egress.upstream.requested", + expect.objectContaining({ + "app.grant.access": "write", + "app.log.audit": true, + }), + ); + }); + it("rejects non-namespaced application event names", async () => { const { logWarn } = await loadLoggingModule();