Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cmd/api/handlers/audit.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import (
//
// ?service=... exact match on service name
// ?username=... case-insensitive partial match on username
// ?type=... log type integer (1..10), see pkg/auditlog.LogType*
// ?type=... log type integer (1..11), see pkg/auditlog.LogType*
// ?env_uuid=... filter to one environment (resolved to internal ID)
// ?since=RFC3339 created_at >= since
// ?until=RFC3339 created_at <= until
Expand Down
2 changes: 1 addition & 1 deletion cmd/api/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -1258,7 +1258,7 @@ func osctrlAPIService() {
log.Info().Bool("writes", flagParams.MCP.AllowWrites).Msgf("MCP enabled — serving %s", _apiPath(apiMCPPath))
muxAPI.Handle(
_apiPath(apiMCPPath),
handlerAuthCheck(mcpHandler(muxAPI, buildVersion, flagParams.MCP.AllowWrites), flagParams.Service.Auth, flagParams.JWT.JWTSecret))
handlerAuthCheck(mcpHandler(muxAPI, buildVersion, flagParams.MCP.AllowWrites, auditLog), flagParams.Service.Auth, flagParams.JWT.JWTSecret))
}
// Launch listeners for API server. The server runs in a goroutine so
// the main goroutine can wait on the restart channel and trigger a
Expand Down
116 changes: 113 additions & 3 deletions cmd/api/mcp.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,35 @@ package main

import (
"bytes"
"context"
"encoding/json"
"fmt"
"io"
"net/http"
"strings"

sdk "github.com/modelcontextprotocol/go-sdk/mcp"

"github.com/jmpsec/osctrl/cmd/api/handlers"
"github.com/jmpsec/osctrl/pkg/apiclient"
"github.com/jmpsec/osctrl/pkg/auditlog"
osctrlmcp "github.com/jmpsec/osctrl/pkg/mcp"
)

// maxAuditedArgsBytes bounds how much of a tool call's arguments the audit
// middleware records. Tool inputs are small scalars and short lists by
// construction (see pkg/mcp's tool schemas); this only guards against a
// pathological or malicious client sending an oversized arguments blob and
// bloating the audit_logs table.
const maxAuditedArgsBytes = 500

// mcpAuditLog is the one audit-log capability the MCP boundary needs.
// Narrowed to a single method — matching the Backend/WriteBackend pattern in
// pkg/mcp — so tests can fake it without a database.
type mcpAuditLog interface {
MCPToolCall(username, tool, args, ip string, envID uint, failed bool)
}

// mcpInternalHost is the host the in-process client addresses. Nothing
// resolves it: loopbackTransport routes on method and path only, and none of
// osctrl-api's mux patterns are host-scoped. It exists because http.Request
Expand Down Expand Up @@ -111,8 +130,24 @@ func (r *responseRecorder) result(req *http.Request) *http.Response {
// The returned handler must still be mounted behind handlerAuthCheck: that
// rejects unauthenticated callers up front, so a bad token fails once at the
// MCP boundary instead of once per tool call.
func mcpHandler(apiHandler http.Handler, version string, allowWrites bool) http.Handler {
//
// auditLog receives one entry per tool call, tagged with auditlog.LogTypeMCP
// so an operator can filter agent activity out of the far larger volume of
// SPA/CLI-driven entries, which carry no origin marker at all. This is only
// possible here, at the hosted boundary: osctrl-api itself is dispatching
// the call, so it has first-hand knowledge the request came from MCP. The
// standalone stdio binary (cmd/mcp) is, from the server's point of view,
// just another authenticated HTTP client — indistinguishable from
// osctrl-cli — and its calls are audited the same way any REST client's
// would be, with no special tagging.
func mcpHandler(apiHandler http.Handler, version string, allowWrites bool, auditLog mcpAuditLog) http.Handler {
getServer := func(r *http.Request) *sdk.Server {
// Captured once per session (getServer runs at session creation, not
// per tool call — see mcp.NewStreamableHTTPHandler), same lifetime
// as the credentials loopbackTransport binds below.
username := mcpCallerUsername(r)
ip := strings.Split(r.RemoteAddr, ":")[0]

// One client per request, bound to that request's credentials. The
// MCP server is cheap to build and holds no state, so per-request
// construction keeps sessions from ever sharing an identity.
Expand All @@ -129,10 +164,85 @@ func mcpHandler(apiHandler http.Handler, version string, allowWrites bool) http.
// or a nil transport, neither of which is reachable here.
return nil
}
var srv *sdk.Server
if allowWrites {
return osctrlmcp.NewServer(client, version, osctrlmcp.WithWrites(client))
srv = osctrlmcp.NewServer(client, version, osctrlmcp.WithWrites(client))
} else {
srv = osctrlmcp.NewServer(client, version)
}
return osctrlmcp.NewServer(client, version)
srv.AddReceivingMiddleware(auditToolCalls(auditLog, username, ip))
return srv
}
return sdk.NewStreamableHTTPHandler(getServer, nil)
}

// mcpCallerUsername reads the username handlerAuthCheck already resolved and
// stashed on the request context before mcpHandler ever runs. Re-deriving it
// here (rather than re-parsing the token) keeps this file agreeing with the
// rest of cmd/api about who the caller is, with no second source of truth.
func mcpCallerUsername(r *http.Request) string {
cv, ok := r.Context().Value(handlers.ContextKey(contextAPI)).(handlers.ContextValue)
if !ok {
return ""
}
return cv["user"]
}

// auditToolCalls records one auditLog.MCPToolCall entry per "tools/call"
// request on this session — every read tool included, matching the existing
// convention of auditing GET-style views (see NodeAction's "viewed all
// nodes" and friends) rather than treating audit volume as a reason to skip
// reads. It never blocks or fails a tool call: audit failures only log a
// warning inside MCPToolCall itself.
func auditToolCalls(auditLog mcpAuditLog, username, ip string) sdk.Middleware {
return func(next sdk.MethodHandler) sdk.MethodHandler {
return func(ctx context.Context, method string, req sdk.Request) (sdk.Result, error) {
result, err := next(ctx, method, req)
if method != "tools/call" {
return result, err
}

tool, args := toolCallDetails(req)
failed := err != nil
// A tool error is reported in-band (IsError on the result, no Go
// error) so the model can see and self-correct — see
// CallToolResult.IsError. That still counts as "failed" for the
// audit trail: a denied run_query is exactly what an operator
// wants to be able to find.
if ctr, ok := result.(*sdk.CallToolResult); ok && ctr.IsError {
failed = true
}
auditLog.MCPToolCall(username, tool, args, ip, auditlog.NoEnvironment, failed)
return result, err
}
}
}

// toolCallDetails extracts the tool name and a size-bounded JSON rendering
// of its arguments from a tools/call request. The concrete parameter type
// varies by SDK dispatch phase (CallToolParamsRaw server-side,
// CallToolParams on the wire-shaped path); both are handled so this never
// depends on an SDK internal staying the same across versions any more than
// necessary. An unrecognized type degrades to an empty summary rather than
// panicking — a middleware must never be the reason a tool call fails.
func toolCallDetails(req sdk.Request) (tool, args string) {
switch p := req.GetParams().(type) {
case *sdk.CallToolParamsRaw:
return p.Name, truncate(string(p.Arguments), maxAuditedArgsBytes)
case *sdk.CallToolParams:
b, err := json.Marshal(p.Arguments)
if err != nil {
return p.Name, ""
}
return p.Name, truncate(string(b), maxAuditedArgsBytes)
default:
return "unknown", ""
}
}

func truncate(s string, n int) string {
if len(s) <= n {
return s
}
return s[:n] + "...(truncated)"
}
85 changes: 83 additions & 2 deletions cmd/api/mcp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,35 @@ func (a *recordingAPI) ServeHTTP(w http.ResponseWriter, r *http.Request) {
_, _ = w.Write([]byte(a.body))
}

type fakeMCPAuditCall struct {
username string
tool string
args string
ip string
envID uint
failed bool
}

type fakeAuditLog struct {
calls []fakeMCPAuditCall
}

func (a *fakeAuditLog) MCPToolCall(username, tool, args, ip string, envID uint, failed bool) {
a.calls = append(a.calls, fakeMCPAuditCall{
username: username,
tool: tool,
args: args,
ip: ip,
envID: envID,
failed: failed,
})
}

// newMCPClient mounts mcpHandler over api and returns a connected MCP session.
// creds are applied to the inbound HTTP request the way a real client would.
func newMCPClient(t *testing.T, api http.Handler, apply func(*http.Request)) *sdk.ClientSession {
t.Helper()
srv := httptest.NewServer(mcpHandler(api, "test", false))
srv := httptest.NewServer(mcpHandler(api, "test", false, &fakeAuditLog{}))
t.Cleanup(srv.Close)

client := sdk.NewClient(&sdk.Implementation{Name: "test", Version: "test"}, nil)
Expand Down Expand Up @@ -193,6 +217,63 @@ func TestPerRequestCredentialIsolation(t *testing.T) {
}
}

func TestAuditToolCallsRecordsToolCallOutcome(t *testing.T) {
for _, tc := range []struct {
name string
result sdk.Result
err error
wantFailed bool
}{
{
name: "success",
result: &sdk.CallToolResult{},
wantFailed: false,
},
{
name: "tool error result",
result: &sdk.CallToolResult{IsError: true},
wantFailed: true,
},
{
name: "protocol error",
err: context.Canceled,
wantFailed: true,
},
} {
t.Run(tc.name, func(t *testing.T) {
auditLog := &fakeAuditLog{}
handler := auditToolCalls(auditLog, "alice", "192.0.2.10")(func(context.Context, string, sdk.Request) (sdk.Result, error) {
return tc.result, tc.err
})
req := &sdk.ServerRequest[*sdk.CallToolParamsRaw]{
Params: &sdk.CallToolParamsRaw{
Name: "get_node",
Arguments: json.RawMessage(`{"environment":"prod","identifier":"node-1"}`),
},
}

_, _ = handler(context.Background(), "tools/call", req)

if len(auditLog.calls) != 1 {
t.Fatalf("audit calls = %d, want 1", len(auditLog.calls))
}
call := auditLog.calls[0]
if call.username != "alice" || call.ip != "192.0.2.10" {
t.Fatalf("audit attribution = %+v, want alice/192.0.2.10", call)
}
if call.tool != "get_node" {
t.Fatalf("tool = %q, want get_node", call.tool)
}
if call.args != `{"environment":"prod","identifier":"node-1"}` {
t.Fatalf("args = %q", call.args)
}
if call.failed != tc.wantFailed {
t.Fatalf("failed = %v, want %v", call.failed, tc.wantFailed)
}
})
}
}

// The hosted handler must honour the operator's write switch: mounting MCP
// does not by itself put mutating tools on the wire.
func TestHostedWriteToolsGated(t *testing.T) {
Expand All @@ -207,7 +288,7 @@ func TestHostedWriteToolsGated(t *testing.T) {
{"writes on", true, true},
} {
t.Run(tc.name, func(t *testing.T) {
srv := httptest.NewServer(mcpHandler(api, "test", tc.allowWrites))
srv := httptest.NewServer(mcpHandler(api, "test", tc.allowWrites, &fakeAuditLog{}))
defer srv.Close()

client := sdk.NewClient(&sdk.Implementation{Name: "test", Version: "test"}, nil)
Expand Down
2 changes: 2 additions & 0 deletions frontend/src/api/audit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ export const LOG_TYPE = {
Setting: 8,
Visit: 9,
User: 10,
MCP: 11,
} as const;

export const LOG_TYPE_LABELS: Record<number, string> = {
Expand All @@ -74,4 +75,5 @@ export const LOG_TYPE_LABELS: Record<number, string> = {
8: 'setting',
9: 'visit',
10: 'user',
11: 'mcp',
};
16 changes: 15 additions & 1 deletion frontend/src/features/audit/AuditPage.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ vi.mock('$/api/client', () => ({
const auditSearchSchema = z.object({
service: z.string().optional(),
username: z.string().optional(),
type: z.number().int().min(1).max(10).optional(),
type: z.number().int().min(1).max(11).optional(),
env_uuid: z.string().optional(),
since: z.string().optional(),
until: z.string().optional(),
Expand Down Expand Up @@ -176,6 +176,20 @@ describe('AuditPage', () => {
expect(lastArgs?.type).toBe(1);
});

it('accepts the MCP audit type filter', async () => {
mockList.mockResolvedValue(makeResp([]));
renderWithProviders(
makeTestRouter('/_app/audit?type=11&page=1'),
);

await waitFor(() => {
expect(mockList).toHaveBeenCalled();
});
const calls = mockList.mock.calls;
const lastArgs = calls[calls.length - 1]?.[0];
expect(lastArgs?.type).toBe(11);
});

it('Apply filters button writes the username draft into the URL', async () => {
const user = userEvent.setup();
mockList.mockResolvedValue(makeResp([]));
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/features/audit/AuditPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ type Search = z.infer<typeof auditSearchSchema>;
// uses bare "tls"/"admin"/"api"). The two should not be unified — audit
// readers compare to what was actually written to the column.
const SERVICES = ['', 'osctrl-tls', 'osctrl-api', 'osctrl-cli'] as const;
const LOG_TYPE_KEYS = [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10] as const;
const LOG_TYPE_KEYS = [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11] as const;

export function AuditPage() {
usePageTitle('Audit');
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/routes/_app/audit.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { AuditPage } from '$/features/audit/AuditPage';
export const auditSearchSchema = z.object({
service: z.string().optional(),
username: z.string().optional(),
type: z.number().int().min(1).max(10).optional(),
type: z.number().int().min(1).max(11).optional(),
env_uuid: z.string().optional(),
since: z.string().optional(),
until: z.string().optional(),
Expand Down
37 changes: 37 additions & 0 deletions pkg/auditlog/audit.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ var LogTypes = map[uint]struct{}{
LogTypeSetting: {},
LogTypeVisit: {},
LogTypeUser: {},
LogTypeMCP: {},
}

// PageFilter describes the inputs accepted by GetPaged.
Expand Down Expand Up @@ -121,6 +122,11 @@ const (
LogTypeSetting = 8
LogTypeVisit = 9
LogTypeUser = 10
// LogTypeMCP marks audit entries generated by an MCP tool call. Kept
// distinct from LogTypeQuery/LogTypeNode/etc. so an operator can filter
// agent activity out of the (much larger) volume of SPA/CLI-driven
// entries, which carry no origin marker at all.
LogTypeMCP = 11
// Severities
SeverityInfo = 1
SeverityWarning = 2
Expand Down Expand Up @@ -334,6 +340,37 @@ func (m *AuditLogManager) NewToken(username, ip string) {
}
}

// MCPToolCall records that an MCP client invoked a tool, tagged with the
// dedicated LogTypeMCP so an operator can filter agent activity out of the
// much larger volume of SPA/CLI-driven entries, none of which carry any
// origin marker.
//
// Recorded independently of whatever the dispatched REST handler itself
// audits for the same action (run_query still produces its own NewQuery
// entry, for example) — this is the complete record of what the agent
// asked for, including every read tool call, which never reaches a
// mutating handler at all.
//
// args is the tool's JSON-encoded input; callers are expected to have
// already bounded its size. failed marks a denied or errored call
// (permission refusal, bad input, a downstream 4xx/5xx) with
// SeverityWarning instead of SeverityInfo, so it stands out the same way
// FailedLogin does next to NewLogin.
func (m *AuditLogManager) MCPToolCall(username, tool, args, ip string, envID uint, failed bool) {
if !m.Enabled {
return
}
severity := uint(SeverityInfo)
line := fmt.Sprintf("agent called %s(%s)", tool, args)
if failed {
severity = SeverityWarning
line = fmt.Sprintf("agent call to %s(%s) was denied or failed", tool, args)
}
if err := m.CreateNew(username, line, ip, LogTypeMCP, severity, envID); err != nil {
log.Err(err).Msg("error creating MCP tool-call audit log")
}
}

// ConfAction - create new configuration action audit log entry
func (m *AuditLogManager) ConfAction(username, action, ip string, envID uint) {
if !m.Enabled {
Expand Down
Loading
Loading