fix(awaitreply): redact await reply responses - #95
Conversation
📝 WalkthroughEnglishOverview
Compatibility and behavioral risks
Recommended validation
中文变更概述
兼容性与行为风险
建议验证步骤
WalkthroughThe await_user_reply tool now redacts sensitive content in error messages and successful AgentName responses, with tests covering authorization tokens and multiple secret-like fields. 中文：工具现在会清理错误信息和成功响应中的敏感内容，并通过测试验证令牌及多种秘密字段的脱敏行为。 ChangesAwait-reply response redaction
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
868f193 to
4cbcd6e
Compare
948d294 to
cc081c0
Compare
cc081c0 to
9e88b28
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tool/awaitreply/await_reply_tool.go (1)
99-105: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winKeep the redaction fallback fail-closed.
Returning
messagewhenplatform.NewRedactor()fails defeats this security boundary and can expose the raw error or agent name. The current no-argument constructor appears not to fail with the shown implementation, but use a safe fixed placeholder or propagate the error instead of failing open.Suggested fail-closed fallback
redactor, err := platform.NewRedactor() if err != nil { - return message + return "****" }中文
确保脱敏失败时采用故障关闭策略。
当
platform.NewRedactor()失败时返回原始message，会绕过该安全边界并泄露原始错误或 agent 名称。根据当前实现，无参数构造函数似乎不会失败，但仍应返回固定安全占位符或传播错误，避免故障时明文泄露。As per path instructions, security boundaries and error semantics must remain safe for untrusted model-visible data.
🤖 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 `@tool/awaitreply/await_reply_tool.go` around lines 99 - 105, Update redactResponseMessage so a failure from platform.NewRedactor() uses a fixed safe placeholder or propagates the error instead of returning the raw message. Preserve redaction for successful initialization and ensure no untrusted content is exposed through the fallback.Source: Path instructions
🤖 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 `@tool/awaitreply/await_reply_tool_test.go`:
- Around line 50-65: Extend TestTool_CallRedactsModelVisibleResponseFields with
end-to-end tests covering the invalid-request response path and the
MarkAwaitingUserReply error path in Call. Assert each returned Message is
redacted while the internally stored route retains the original unredacted agent
name, using the existing test setup and externally observable behavior rather
than testing helper implementation details.
---
Nitpick comments:
In `@tool/awaitreply/await_reply_tool.go`:
- Around line 99-105: Update redactResponseMessage so a failure from
platform.NewRedactor() uses a fixed safe placeholder or propagates the error
instead of returning the raw message. Preserve redaction for successful
initialization and ensure no untrusted content is exposed through the fallback.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 0193c0ca-af44-49ab-92f9-f0bd5b1e1450
📒 Files selected for processing (2)
tool/awaitreply/await_reply_tool.gotool/awaitreply/await_reply_tool_test.go
| func TestTool_CallRedactsModelVisibleResponseFields(t *testing.T) { | ||
| tl := New() | ||
| inv := &agent.Invocation{ | ||
| AgentName: "clarifier Authorization: ApiKey raw-token", | ||
| } | ||
| ctx := agent.NewInvocationContext(context.Background(), inv) | ||
|
|
||
| got, err := tl.Call(ctx, []byte(`{}`)) | ||
| require.NoError(t, err) | ||
|
|
||
| resp, ok := got.(Response) | ||
| require.True(t, ok) | ||
| require.True(t, resp.Success) | ||
| require.NotContains(t, resp.AgentName, "raw-token") | ||
| require.Contains(t, resp.AgentName, "Authorization: ****") | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Cover every Call response path, not only the helper.
These tests verify successful AgentName redaction and helper patterns, but they would still pass if the invalid-request branch at Line 73 or the MarkAwaitingUserReply error branch at Line 89 stopped redacting Message. Add end-to-end tests for both branches, and assert that the stored route still contains the original unredacted agent name.
As per path instructions, tests should prioritize external behavior, boundary/error paths, and regression scenarios over implementation details.
中文
覆盖 Call 的所有响应路径，而不仅是辅助函数。
当前测试验证了成功响应中的 AgentName 脱敏和辅助函数的规则，但即使 Line 73 的非法请求分支或 Line 89 的 MarkAwaitingUserReply 错误分支停止脱敏，测试仍可能通过。请为这两个分支增加端到端测试，并断言内部保存的路由仍使用原始、未脱敏的 agent 名称。
Also applies to: 110-129
🤖 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 `@tool/awaitreply/await_reply_tool_test.go` around lines 50 - 65, Extend
TestTool_CallRedactsModelVisibleResponseFields with end-to-end tests covering
the invalid-request response path and the MarkAwaitingUserReply error path in
Call. Assert each returned Message is redacted while the internally stored route
retains the original unredacted agent name, using the existing test setup and
externally observable behavior rather than testing helper implementation
details.
Source: Path instructions
2 participants
Objective
Close one Phase2 model-visible response redaction gap in the
await_user_replyframework tool.Changes
await_user_reply.MarkAwaitingUserReplyfailure messages before returning them as tool responses.agent_namefield to avoid exposing sensitive agent labels.Validation
go test ./tool/awaitreplygo vet ./tool/awaitreplygit diff --checkKnown Risks / Limitations
await_user_replytool response fields.Follow-up