Skip to content

Commit f4f7e23

Browse files
committed
refactor(tools): cap window at 90d, guard TimeWindow methods on zero, and expand test coverage
1 parent abb6af4 commit f4f7e23

3 files changed

Lines changed: 142 additions & 8 deletions

File tree

internal/infra/mcp/tools/tool_k8s_list_top_http_errors_in_pods_test.go

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,4 +160,45 @@ var _ = Describe("KubernetesListTopHttpErrorsInPods Tool", func() {
160160
Expect(result.Content[0].(mcp.TextContent).Text).To(ContainSubstring("invalid interval format"))
161161
})
162162
})
163+
164+
When("the time window is invalid", func() {
165+
It("rejects an unparseable start", func() {
166+
serverTool := mcpServer.GetTool("k8s_list_top_http_errors_in_pods")
167+
result, err := serverTool.Handler(ctx, mcp.CallToolRequest{
168+
Params: mcp.CallToolParams{
169+
Name: "k8s_list_top_http_errors_in_pods",
170+
Arguments: map[string]any{"start": "not-a-timestamp"},
171+
},
172+
})
173+
Expect(err).NotTo(HaveOccurred())
174+
Expect(result.IsError).To(BeTrue())
175+
})
176+
177+
It("rejects an end before start", func() {
178+
serverTool := mcpServer.GetTool("k8s_list_top_http_errors_in_pods")
179+
result, err := serverTool.Handler(ctx, mcp.CallToolRequest{
180+
Params: mcp.CallToolParams{
181+
Name: "k8s_list_top_http_errors_in_pods",
182+
Arguments: map[string]any{
183+
"start": "2026-04-16T11:00:00Z",
184+
"end": "2026-04-16T10:00:00Z",
185+
},
186+
},
187+
})
188+
Expect(err).NotTo(HaveOccurred())
189+
Expect(result.IsError).To(BeTrue())
190+
})
191+
192+
It("rejects an end provided without start", func() {
193+
serverTool := mcpServer.GetTool("k8s_list_top_http_errors_in_pods")
194+
result, err := serverTool.Handler(ctx, mcp.CallToolRequest{
195+
Params: mcp.CallToolParams{
196+
Name: "k8s_list_top_http_errors_in_pods",
197+
Arguments: map[string]any{"end": "2026-04-16T11:00:00Z"},
198+
},
199+
})
200+
Expect(err).NotTo(HaveOccurred())
201+
Expect(result.IsError).To(BeTrue())
202+
})
203+
})
163204
})

internal/infra/mcp/tools/utils.go

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -59,9 +59,10 @@ func RequiredPermissionsFromTool(tool mcp.Tool) []string {
5959

6060
const (
6161
windowedQueryTimeout = "60s"
62+
maxWindowDuration = 90 * 24 * time.Hour
6263
timeParamStart = "start"
6364
timeParamEnd = "end"
64-
startParamDescription = "Start of the query window as an RFC3339 timestamp (e.g. 2026-04-01T00:00:00Z). When omitted, the tool returns an instant snapshot (current behavior). When provided without end, end defaults to now."
65+
startParamDescription = "Start of the query window as an RFC3339 timestamp (e.g. 2026-04-01T00:00:00Z). When omitted, the tool returns an instant snapshot (current behavior). When provided without end, end defaults to now. The window between start and end may not exceed 90 days."
6566
endParamDescription = "End of the query window as an RFC3339 timestamp (e.g. 2026-04-01T01:00:00Z). Requires start. If in the future, clamped to now."
6667
)
6768

@@ -75,10 +76,16 @@ func (w TimeWindow) IsZero() bool {
7576
}
7677

7778
func (w TimeWindow) RangeSelector() string {
79+
if w.IsZero() {
80+
panic("RangeSelector called on zero TimeWindow")
81+
}
7882
return fmt.Sprintf("[%ds]", int64(w.End.Sub(w.Start).Seconds()))
7983
}
8084

8185
func (w TimeWindow) WindowSeconds() int64 {
86+
if w.IsZero() {
87+
panic("WindowSeconds called on zero TimeWindow")
88+
}
8289
return int64(w.End.Sub(w.Start).Seconds())
8390
}
8491

@@ -94,15 +101,16 @@ func (w TimeWindow) EvalTime() (*sysdig.Time, error) {
94101
}
95102

96103
func (w TimeWindow) ApplyToParams(params *sysdig.GetQueryV1Params) error {
104+
if w.IsZero() {
105+
return nil
106+
}
97107
evalTime, err := w.EvalTime()
98108
if err != nil {
99109
return err
100110
}
101111
params.Time = evalTime
102-
if !w.IsZero() {
103-
timeout := sysdig.Timeout(windowedQueryTimeout)
104-
params.Timeout = &timeout
105-
}
112+
timeout := sysdig.Timeout(windowedQueryTimeout)
113+
params.Timeout = &timeout
106114
return nil
107115
}
108116

@@ -114,7 +122,6 @@ func WithTimeWindowParams() mcp.ToolOption {
114122
}
115123

116124
// Reads "start" and "end" from the request, validates them, and return the resolved TimeWindow.
117-
118125
func ParseTimeWindow(request mcp.CallToolRequest, clk clock.Clock) (TimeWindow, error) {
119126
startStr := mcp.ParseString(request, timeParamStart, "")
120127
endStr := mcp.ParseString(request, timeParamEnd, "")
@@ -150,6 +157,10 @@ func ParseTimeWindow(request mcp.CallToolRequest, clk clock.Clock) (TimeWindow,
150157
return TimeWindow{}, fmt.Errorf("end (%s) must be after start (%s)", end.Format(time.RFC3339), start.Format(time.RFC3339))
151158
}
152159

160+
if end.Sub(start) > maxWindowDuration {
161+
return TimeWindow{}, fmt.Errorf("window between start (%s) and end (%s) exceeds the maximum of 90 days", start.Format(time.RFC3339), end.Format(time.RFC3339))
162+
}
163+
153164
return TimeWindow{Start: start, End: end}, nil
154165
}
155166

internal/infra/mcp/tools/utils_test.go

Lines changed: 84 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010

1111
mocks_clock "github.com/sysdiglabs/sysdig-mcp-server/internal/infra/clock/mocks"
1212
"github.com/sysdiglabs/sysdig-mcp-server/internal/infra/mcp/tools"
13+
"github.com/sysdiglabs/sysdig-mcp-server/internal/infra/sysdig"
1314
)
1415

1516
var _ = Describe("ParseTimeWindow", func() {
@@ -101,11 +102,92 @@ var _ = Describe("ParseTimeWindow", func() {
101102
Expect(tw.End).To(Equal(time.Date(2026, time.April, 16, 11, 0, 0, 0, time.UTC)))
102103
})
103104

104-
It("truncates sub-second precision from now so RangeSelector never emits [0s]", func() {
105+
It("rejects start at or after the truncated current second (end is not after start)", func() {
105106
mockClock.EXPECT().Now().Return(now)
106107
_, err := tools.ParseTimeWindow(makeRequest(map[string]any{
107108
"start": "2026-04-16T12:00:00Z",
108109
}), mockClock)
109-
Expect(err).To(HaveOccurred())
110+
Expect(err).To(MatchError(ContainSubstring("must be after start")))
111+
})
112+
113+
It("rejects windows longer than 90 days", func() {
114+
mockClock.EXPECT().Now().Return(now)
115+
_, err := tools.ParseTimeWindow(makeRequest(map[string]any{
116+
"start": "2026-01-01T00:00:00Z",
117+
"end": "2026-04-30T00:00:00Z",
118+
}), mockClock)
119+
Expect(err).To(MatchError(ContainSubstring("exceeds the maximum of 90 days")))
120+
})
121+
122+
It("accepts windows of exactly 90 days", func() {
123+
mockClock.EXPECT().Now().Return(now)
124+
_, err := tools.ParseTimeWindow(makeRequest(map[string]any{
125+
"start": "2026-01-01T00:00:00Z",
126+
"end": "2026-04-01T00:00:00Z",
127+
}), mockClock)
128+
Expect(err).NotTo(HaveOccurred())
129+
})
130+
})
131+
132+
var _ = Describe("TimeWindow methods", func() {
133+
var (
134+
start = time.Date(2026, time.April, 16, 10, 0, 0, 0, time.UTC)
135+
end = time.Date(2026, time.April, 16, 11, 0, 0, 0, time.UTC)
136+
tw = tools.TimeWindow{Start: start, End: end}
137+
)
138+
139+
Describe("RangeSelector", func() {
140+
It("returns the PromQL range-selector literal in seconds for a 1h window", func() {
141+
Expect(tw.RangeSelector()).To(Equal("[3600s]"))
142+
})
143+
144+
It("panics when called on a zero TimeWindow", func() {
145+
Expect(func() { tools.TimeWindow{}.RangeSelector() }).To(PanicWith("RangeSelector called on zero TimeWindow"))
146+
})
147+
})
148+
149+
Describe("WindowSeconds", func() {
150+
It("returns the window length in whole seconds for a 1h window", func() {
151+
Expect(tw.WindowSeconds()).To(Equal(int64(3600)))
152+
})
153+
154+
It("panics when called on a zero TimeWindow", func() {
155+
Expect(func() { tools.TimeWindow{}.WindowSeconds() }).To(PanicWith("WindowSeconds called on zero TimeWindow"))
156+
})
157+
})
158+
159+
Describe("EvalTime", func() {
160+
It("returns nil eval time for a zero TimeWindow", func() {
161+
et, err := tools.TimeWindow{}.EvalTime()
162+
Expect(err).NotTo(HaveOccurred())
163+
Expect(et).To(BeNil())
164+
})
165+
166+
It("returns a sysdig.Time encoding the window's End for a non-zero window", func() {
167+
et, err := tw.EvalTime()
168+
Expect(err).NotTo(HaveOccurred())
169+
Expect(et).NotTo(BeNil())
170+
171+
var expected sysdig.Time
172+
Expect(expected.FromQueryTime1(end.Unix())).To(Succeed())
173+
Expect(*et).To(Equal(expected))
174+
})
175+
})
176+
177+
Describe("ApplyToParams", func() {
178+
It("leaves params untouched on a zero TimeWindow", func() {
179+
params := &sysdig.GetQueryV1Params{}
180+
Expect(tools.TimeWindow{}.ApplyToParams(params)).To(Succeed())
181+
Expect(params.Time).To(BeNil())
182+
Expect(params.Timeout).To(BeNil())
183+
})
184+
185+
It("sets Time and Timeout for a non-zero window", func() {
186+
params := &sysdig.GetQueryV1Params{}
187+
Expect(tw.ApplyToParams(params)).To(Succeed())
188+
Expect(params.Time).NotTo(BeNil())
189+
Expect(params.Timeout).NotTo(BeNil())
190+
Expect(*params.Timeout).To(Equal(sysdig.Timeout("60s")))
191+
})
110192
})
111193
})

0 commit comments

Comments
 (0)