mirror of
https://github.com/infiniflow/ragflow.git
synced 2026-08-03 06:17:29 +08:00
Port 14 upstream agent security / correctness fixes to Go canvas (#16455)
Mirrors 14 merged upstream PRs into the Go agent port. PRs ported: - #15609 ExeSQL SSRF guard + DNS pin - #15436 HTTP timeout on external API tools - #16363 be_output restore + DeepL error path - #15644 switch no longer matches empty condition - #15374 session_id bind to path agent_id (DAO idor guard) - #16169 sandbox artifact ownership gate - #15457 tenant ownership on agentbots - #15145 rerun agent document access check - #15446 thinking switch (component portion; provider policy lives in internal/llm) - #15426 Invoke URL/proxy SSRF + DNS pin + no-redirects - #15238 agentbot thinking-logs beta endpoint - #14589 UserFillUp SSE event propagation - #14890 anonymous webhook opt-in - #15068 PipelineChunker new component (text/file_ref/parser_id dispatch; file-format extraction is a follow-up) 40 files, +2355 / -58 lines. 33 new tests, all targeted package suites pass (1721 + 4 skipped); 1 pre-existing flaky test unrelated.
This commit is contained in:
@@ -68,12 +68,38 @@ type chatAgentService interface {
|
||||
RunAgent(ctx context.Context, userID, canvasID, sessionID, version string, userInput any) (<-chan canvas.RunEvent, error)
|
||||
}
|
||||
|
||||
// documentAccessChecker is the minimal surface RerunAgent needs
|
||||
// from DocumentService. Defined as an interface (instead of taking
|
||||
// the concrete *service.DocumentService) so handler tests can
|
||||
// inject a deny-all stub without spinning up the full service
|
||||
// (DB DAOs, storage clients, …). The production *service.DocumentService
|
||||
// satisfies this interface because its Accessible signature
|
||||
// matches.
|
||||
type documentAccessChecker interface {
|
||||
Accessible(docID, userID string) bool
|
||||
}
|
||||
|
||||
// AgentHandler agent handler
|
||||
type AgentHandler struct {
|
||||
agentService *service.AgentService
|
||||
chatRunner chatAgentService
|
||||
fileService agentFileService
|
||||
loader canvasLoader
|
||||
// documentService is optional. Wired in cmd/server_main.go after
|
||||
// NewAgentHandler (which doesn't take it to preserve the existing
|
||||
// test-friendly signature). When nil, RerunAgent falls back to
|
||||
// tenant-only authorization (i.e. cannot verify the doc, so the
|
||||
// check is skipped — same shape as the pre-port behaviour).
|
||||
documentService documentAccessChecker
|
||||
}
|
||||
|
||||
// WithDocumentService injects the document service used by
|
||||
// RerunAgent to enforce DocumentService.accessible(docID, tenantID)
|
||||
// before re-running. Returns the receiver for chaining in
|
||||
// server_main wiring.
|
||||
func (h *AgentHandler) WithDocumentService(s documentAccessChecker) *AgentHandler {
|
||||
h.documentService = s
|
||||
return h
|
||||
}
|
||||
|
||||
// NewAgentHandler create agent handler
|
||||
@@ -1088,8 +1114,19 @@ func (h *AgentHandler) AgentChatCompletions(c *gin.Context) {
|
||||
// yet; we keep the validation envelope (101 with the "required
|
||||
// argument are missing" message) so the test contract is satisfied,
|
||||
// and accept the request when all three fields are present.
|
||||
//
|
||||
// Tenant / document ownership gate (PR #15145, review round 6):
|
||||
// body.id is treated as a document ID and
|
||||
// `DocumentService.accessible(docID, user.ID)` is enforced BEFORE
|
||||
// the rerun. The gate is REQUIRED: a nil documentService turns a
|
||||
// wiring miss into an auth bypass (any caller could rerun an
|
||||
// arbitrary doc id without an ownership check), so we fail closed
|
||||
// with 500 instead of accepting the request. On denial we return
|
||||
// "Document not found." so a caller cannot probe whether a
|
||||
// document exists in another tenant.
|
||||
func (h *AgentHandler) RerunAgent(c *gin.Context) {
|
||||
if _, code, msg := GetUser(c); code != common.CodeSuccess {
|
||||
user, code, msg := GetUser(c)
|
||||
if code != common.CodeSuccess {
|
||||
jsonError(c, code, msg)
|
||||
return
|
||||
}
|
||||
@@ -1117,6 +1154,20 @@ func (h *AgentHandler) RerunAgent(c *gin.Context) {
|
||||
"required argument are missing: "+strings.Join(missing, ",")+"; ")
|
||||
return
|
||||
}
|
||||
// Fail closed on missing dependency: a nil documentService
|
||||
// means the handler was wired without the access checker,
|
||||
// which would let any caller rerun an arbitrary doc id
|
||||
// without proving ownership. Surface as a 500 so a missing
|
||||
// dependency is loud, not silent.
|
||||
if h.documentService == nil {
|
||||
zap.L().Error("RerunAgent: documentService is nil; refusing request to prevent auth bypass")
|
||||
jsonError(c, common.CodeServerError, "server misconfiguration: document service not wired")
|
||||
return
|
||||
}
|
||||
if !h.documentService.Accessible(body.ID, user.ID) {
|
||||
jsonError(c, common.CodeDataError, "Document not found.")
|
||||
return
|
||||
}
|
||||
c.JSON(http.StatusOK, gin.H{
|
||||
"code": common.CodeSuccess,
|
||||
"data": true,
|
||||
|
||||
@@ -955,7 +955,12 @@ func TestRerunAgent_RequiresAllFields(t *testing.T) {
|
||||
}
|
||||
|
||||
// TestRerunAgent_AcceptsCompleteRequest covers the happy path: all
|
||||
// three required fields present -> 200 / code 0.
|
||||
// three required fields present + documentService wired with an
|
||||
// accessible document -> 200 / code 0.
|
||||
//
|
||||
// Round 6: now that RerunAgent fails closed when documentService is
|
||||
// nil, the happy path needs an accessible stub. We use the deny-all
|
||||
// stub flipped to accessible=true so the gate passes.
|
||||
func TestRerunAgent_AcceptsCompleteRequest(t *testing.T) {
|
||||
gin.SetMode(gin.TestMode)
|
||||
w := httptest.NewRecorder()
|
||||
@@ -966,13 +971,15 @@ func TestRerunAgent_AcceptsCompleteRequest(t *testing.T) {
|
||||
c.Set("user", &entity.User{ID: "u1"})
|
||||
c.Set("user_id", "u1")
|
||||
|
||||
h := NewAgentHandler(service.NewAgentService(), nil)
|
||||
stub := &stubDocService{accessible: true}
|
||||
h := NewAgentHandler(service.NewAgentService(), nil).
|
||||
WithDocumentService(stub)
|
||||
h.RerunAgent(c)
|
||||
|
||||
var resp map[string]interface{}
|
||||
_ = json.Unmarshal(w.Body.Bytes(), &resp)
|
||||
if code, _ := resp["code"].(float64); code != float64(common.CodeSuccess) {
|
||||
t.Errorf("code = %v, want 0", code)
|
||||
t.Errorf("code = %v, want 0 (msg=%v)", code, resp["message"])
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1042,3 +1049,84 @@ func TestGetAgentWebhookLogsReturnsEmptyPoll(t *testing.T) {
|
||||
t.Errorf("missing next_since_ts key")
|
||||
}
|
||||
}
|
||||
|
||||
// TestRerunAgent_RejectsInaccessibleDocument mirrors PR #15145:
|
||||
// POST /api/v1/agents/rerun gates on DocumentService.accessible
|
||||
// (the python "is the document reachable by this tenant" check)
|
||||
// before accepting the request. Without documentService wired,
|
||||
// the gate is skipped (existing behaviour, returns success). With
|
||||
// it wired, an inaccessible doc must return CodeDataError + "Document
|
||||
// not found." so a caller cannot probe whether a doc exists in
|
||||
// another tenant.
|
||||
func TestRerunAgent_RejectsInaccessibleDocument(t *testing.T) {
|
||||
gin.SetMode(gin.TestMode)
|
||||
w := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(w)
|
||||
c.Request = httptest.NewRequest("POST", "/api/v1/agents/rerun",
|
||||
strings.NewReader(`{"id":"doc-victim","dsl":{"path":[]},"component_id":"c1"}`))
|
||||
c.Request.Header.Set("Content-Type", "application/json")
|
||||
c.Set("user", &entity.User{ID: "u1"})
|
||||
c.Set("user_id", "u1")
|
||||
|
||||
// Wire a stub documentService that denies all access. The setter
|
||||
// now accepts a narrow documentAccessChecker interface (PR review
|
||||
// round 5), so the deny-all stub injects cleanly without standing
|
||||
// up the real DocumentService (DB, storage, ...).
|
||||
stub := &stubDocService{accessible: false}
|
||||
h := NewAgentHandler(service.NewAgentService(), nil).
|
||||
WithDocumentService(stub)
|
||||
h.RerunAgent(c)
|
||||
|
||||
var resp map[string]interface{}
|
||||
_ = json.Unmarshal(w.Body.Bytes(), &resp)
|
||||
if code, _ := resp["code"].(float64); code != float64(common.CodeDataError) {
|
||||
t.Errorf("deny-all stub: want code %d (Document not found), got %v (msg=%v)",
|
||||
common.CodeDataError, code, resp["message"])
|
||||
}
|
||||
if msg, _ := resp["message"].(string); !strings.Contains(msg, "Document not found") {
|
||||
t.Errorf("deny-all stub: want message to contain 'Document not found', got %q", msg)
|
||||
}
|
||||
}
|
||||
|
||||
// TestRerunAgent_NoDocumentServiceFailsClosed pins PR review round 6,
|
||||
// Major #2: a nil documentService is now treated as a wiring
|
||||
// misconfiguration that would create an auth bypass, NOT a
|
||||
// backward-compatible "skip the gate" state. The handler must
|
||||
// return 500 / "server misconfiguration" so a missing
|
||||
// dependency is loud and gets fixed, instead of silently
|
||||
// allowing any caller to rerun an arbitrary doc id.
|
||||
func TestRerunAgent_NoDocumentServiceFailsClosed(t *testing.T) {
|
||||
gin.SetMode(gin.TestMode)
|
||||
w := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(w)
|
||||
c.Request = httptest.NewRequest("POST", "/api/v1/agents/rerun",
|
||||
strings.NewReader(`{"id":"doc-anything","dsl":{"path":[]},"component_id":"c1"}`))
|
||||
c.Request.Header.Set("Content-Type", "application/json")
|
||||
c.Set("user", &entity.User{ID: "u1"})
|
||||
c.Set("user_id", "u1")
|
||||
|
||||
h := NewAgentHandler(service.NewAgentService(), nil)
|
||||
// Note: no WithDocumentService call → documentService is nil.
|
||||
// Production wiring (cmd/server_main.go) always calls
|
||||
// WithDocumentService; a nil here means the handler was
|
||||
// constructed without its required dependency.
|
||||
h.RerunAgent(c)
|
||||
|
||||
var resp map[string]interface{}
|
||||
_ = json.Unmarshal(w.Body.Bytes(), &resp)
|
||||
if code, _ := resp["code"].(float64); code != float64(common.CodeServerError) {
|
||||
t.Errorf("nil documentService: want code %d (fail closed), got %v (msg=%v)",
|
||||
common.CodeServerError, code, resp["message"])
|
||||
}
|
||||
if msg, _ := resp["message"].(string); !strings.Contains(msg, "server misconfiguration") {
|
||||
t.Errorf("nil documentService: want message to mention misconfiguration, got %q", msg)
|
||||
}
|
||||
}
|
||||
|
||||
type stubDocService struct {
|
||||
accessible bool
|
||||
}
|
||||
|
||||
func (s *stubDocService) Accessible(_, _ string) bool {
|
||||
return s.accessible
|
||||
}
|
||||
|
||||
@@ -36,6 +36,7 @@ package handler
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"fmt"
|
||||
"net"
|
||||
"strconv"
|
||||
@@ -75,8 +76,26 @@ const (
|
||||
jwtReservedClaims = "exp,sub,aud,iss,nbf,iat"
|
||||
)
|
||||
|
||||
// validateWebhookSecurity is the orchestrator. Empty/nil cfg → no-op
|
||||
// (matches agent_api.py:1607 "No security config → allowed by default").
|
||||
// errWebhookFailClosed is the sentinel returned from BOTH the
|
||||
// "missing security block" branch and the "auth_type=none without
|
||||
// allow_anonymous opt-in" branch of validateWebhookSecurity.
|
||||
// Sharing one error prevents a probe from distinguishing the two
|
||||
// states (and therefore from learning whether a canvas has any
|
||||
// security config at all) — the whole point of PR #14890's
|
||||
// fail-closed default. PR review round 5 (#2) — the previous
|
||||
// form leaked that distinction via two different messages.
|
||||
var errWebhookFailClosed = errors.New(
|
||||
"webhook security is required. Set allow_anonymous to true to permit unauthenticated webhooks.",
|
||||
)
|
||||
|
||||
// validateWebhookSecurity is the orchestrator.
|
||||
//
|
||||
// PR #14890 changed the python default: empty/nil security cfg
|
||||
// is no longer "allowed by default" — it must be a non-empty
|
||||
// dict, and `auth_type == "none"` requires an explicit
|
||||
// `allow_anonymous: true` opt-in. The previous "fail open" default
|
||||
// let unauthenticated callers hit any webhook by simply omitting
|
||||
// the security block.
|
||||
//
|
||||
// Sub-validators run in the python-defined order:
|
||||
// 1. validateMaxBodySize
|
||||
@@ -89,7 +108,7 @@ func validateWebhookSecurity(
|
||||
canvasID string,
|
||||
) error {
|
||||
if len(securityCfg) == 0 {
|
||||
return nil
|
||||
return errWebhookFailClosed
|
||||
}
|
||||
if err := validateMaxBodySize(c, securityCfg); err != nil {
|
||||
return err
|
||||
@@ -275,11 +294,20 @@ func validateRateLimit(canvasID string, cfg map[string]any) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
// validateAuth dispatches on auth_type. Empty cfg or auth_type=="none"
|
||||
// → allow (matches agent_api.py:1621).
|
||||
// validateAuth dispatches on auth_type. `auth_type == "none"`
|
||||
// (or unset) used to allow every request by default — a fail-open
|
||||
// security posture. PR #14890 closed that gap: anonymous
|
||||
// webhook access is now allowed only when the operator sets
|
||||
// `allow_anonymous: true` on the security block (mirrors
|
||||
// python agent_api.py:1659-1664).
|
||||
func validateAuth(c *gin.Context, cfg map[string]any) error {
|
||||
authType, _ := cfg["auth_type"].(string)
|
||||
if authType == "" || authType == "none" {
|
||||
if !isTruthyAllowAnonymous(cfg) {
|
||||
// Same sentinel as the missing-security-block branch
|
||||
// above; see errWebhookFailClosed. PR review round 5 (#2).
|
||||
return errWebhookFailClosed
|
||||
}
|
||||
return nil
|
||||
}
|
||||
switch authType {
|
||||
@@ -293,6 +321,38 @@ func validateAuth(c *gin.Context, cfg map[string]any) error {
|
||||
return fmt.Errorf("unsupported auth_type: %s", authType)
|
||||
}
|
||||
|
||||
// isTruthyAllowAnonymous mirrors python agent_api.py:_is_truthy
|
||||
// applied to cfg["allow_anonymous"]. Returns true only when the
|
||||
// value is an explicit boolean true, a non-zero int, or one of
|
||||
// {"1","true","yes","on"} (case-insensitive, trimmed). Anything
|
||||
// else (including the key being absent) is falsy — closing the
|
||||
// implicit-anonymous gap.
|
||||
func isTruthyAllowAnonymous(cfg map[string]any) bool {
|
||||
if cfg == nil {
|
||||
return false
|
||||
}
|
||||
v, ok := cfg["allow_anonymous"]
|
||||
if !ok {
|
||||
return false
|
||||
}
|
||||
switch x := v.(type) {
|
||||
case bool:
|
||||
return x
|
||||
case int:
|
||||
return x != 0
|
||||
case int64:
|
||||
return x != 0
|
||||
case float64:
|
||||
return x != 0
|
||||
case string:
|
||||
switch strings.ToLower(strings.TrimSpace(x)) {
|
||||
case "1", "true", "yes", "on":
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
// validateTokenAuth mirrors python agent_api.py:1725-1733.
|
||||
//
|
||||
// An empty configured `token_value` previously meant "accept any
|
||||
|
||||
@@ -21,6 +21,8 @@ import (
|
||||
"crypto/rsa"
|
||||
"crypto/x509"
|
||||
"encoding/pem"
|
||||
"errors"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"strings"
|
||||
"testing"
|
||||
@@ -108,11 +110,19 @@ func TestValidateIPWhitelist_RejectForeign(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestValidateAuth_NoneIsAllow covers the auth_type=="none" no-op.
|
||||
func TestValidateAuth_NoneIsAllow(t *testing.T) {
|
||||
// TestValidateAuth_NoneRequiresOptIn covers the auth_type=="none"
|
||||
// opt-in: the old "fail open" default was closed by PR #14890.
|
||||
// An anonymous webhook must explicitly set allow_anonymous=true
|
||||
// to pass; the bare {"auth_type":"none"} block now rejects.
|
||||
func TestValidateAuth_NoneRequiresOptIn(t *testing.T) {
|
||||
c := securityCtx(t, "1.2.3.4:0", nil)
|
||||
if err := validateAuth(c, map[string]any{"auth_type": "none"}); err != nil {
|
||||
t.Errorf("auth_type=none: err = %v, want nil", err)
|
||||
// Bare auth_type=none → must reject (no opt-in).
|
||||
if err := validateAuth(c, map[string]any{"auth_type": "none"}); err == nil {
|
||||
t.Errorf("bare auth_type=none: want error (no opt-in), got nil")
|
||||
}
|
||||
// With explicit allow_anonymous=true → must pass.
|
||||
if err := validateAuth(c, map[string]any{"auth_type": "none", "allow_anonymous": true}); err != nil {
|
||||
t.Errorf("opt-in auth_type=none + allow_anonymous=true: err = %v, want nil", err)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -378,3 +388,89 @@ func TestValidateTokenAuth_EmptyValueRejected(t *testing.T) {
|
||||
t.Errorf("empty token_value: err = nil, want error")
|
||||
}
|
||||
}
|
||||
|
||||
// TestValidateWebhookSecurity_RejectsEmptyConfig guards PR
|
||||
// #14890: empty / nil security config used to be allowed by
|
||||
// default (fail-open), letting unauthenticated webhooks fire on
|
||||
// any canvas. The fix requires an explicit opt-in via
|
||||
// allow_anonymous=true. The handler must return the same generic
|
||||
// error the python fix uses, so a probe cannot distinguish
|
||||
// "missing config" from "exists but no allow_anonymous".
|
||||
func TestValidateWebhookSecurity_RejectsEmptyConfig(t *testing.T) {
|
||||
if err := validateWebhookSecurity(map[string]any{}, newSecurityCtx("c1"), "c1"); err == nil {
|
||||
t.Fatal("empty config: want error, got nil")
|
||||
}
|
||||
if err := validateWebhookSecurity(nil, newSecurityCtx("c1"), "c1"); err == nil {
|
||||
t.Fatal("nil config: want error, got nil")
|
||||
}
|
||||
}
|
||||
|
||||
// TestValidateWebhookSecurity_RejectsAnonymousWithoutOptIn covers
|
||||
// the auth_type=none case without allow_anonymous — used to be
|
||||
// allowed silently. Must be rejected.
|
||||
func TestValidateWebhookSecurity_RejectsAnonymousWithoutOptIn(t *testing.T) {
|
||||
cases := []map[string]any{
|
||||
{"auth_type": "none"},
|
||||
{"auth_type": "none", "allow_anonymous": false},
|
||||
{"auth_type": "none", "allow_anonymous": "false"},
|
||||
{"auth_type": ""},
|
||||
{"auth_type": "", "allow_anonymous": "yes please"},
|
||||
}
|
||||
for _, cfg := range cases {
|
||||
if err := validateWebhookSecurity(cfg, newSecurityCtx("c1"), "c1"); err == nil {
|
||||
t.Errorf("cfg %v: want error, got nil", cfg)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestValidateWebhookSecurity_FailClosedSameError pins PR review
|
||||
// round 5 (#2): the two fail-closed branches (empty security block
|
||||
// vs. anonymous-without-opt-in) MUST return the same error so a
|
||||
// probe cannot distinguish them. Using errors.Is lets the test
|
||||
// survive cosmetic wording tweaks; the assertion is on identity.
|
||||
func TestValidateWebhookSecurity_FailClosedSameError(t *testing.T) {
|
||||
missing := validateWebhookSecurity(map[string]any{}, newSecurityCtx("c1"), "c1")
|
||||
if missing == nil {
|
||||
t.Fatal("empty cfg: want errWebhookFailClosed, got nil")
|
||||
}
|
||||
anon := validateWebhookSecurity(map[string]any{"auth_type": "none"}, newSecurityCtx("c1"), "c1")
|
||||
if anon == nil {
|
||||
t.Fatal("auth_type=none: want errWebhookFailClosed, got nil")
|
||||
}
|
||||
if missing.Error() != anon.Error() {
|
||||
t.Errorf("fail-closed branches must share one error string\n"+
|
||||
" missing-config: %q\n"+
|
||||
" anonymous: %q", missing.Error(), anon.Error())
|
||||
}
|
||||
if !errors.Is(missing, errWebhookFailClosed) || !errors.Is(anon, errWebhookFailClosed) {
|
||||
t.Errorf("both branches must be errors.Is(errWebhookFailClosed); missing=%v anon=%v", missing, anon)
|
||||
}
|
||||
}
|
||||
|
||||
// TestValidateWebhookSecurity_AllowsAnonymousWithOptIn is the
|
||||
// positive control: auth_type=none with an explicit
|
||||
// allow_anonymous=true must pass. The python frontend now
|
||||
// serialises this when the user picks "None" auth.
|
||||
func TestValidateWebhookSecurity_AllowsAnonymousWithOptIn(t *testing.T) {
|
||||
cases := []map[string]any{
|
||||
{"auth_type": "none", "allow_anonymous": true},
|
||||
{"auth_type": "none", "allow_anonymous": "true"},
|
||||
{"auth_type": "none", "allow_anonymous": "1"},
|
||||
{"auth_type": "none", "allow_anonymous": "yes"},
|
||||
{"auth_type": "none", "allow_anonymous": "on"},
|
||||
}
|
||||
for _, cfg := range cases {
|
||||
if err := validateWebhookSecurity(cfg, newSecurityCtx("c1"), "c1"); err != nil {
|
||||
t.Errorf("cfg %v: want nil, got %v", cfg, err)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// newSecurityCtx is a tiny helper that builds a *gin.Context with
|
||||
// just enough request surface for validateWebhookSecurity to run
|
||||
// without panicking on a nil receiver.
|
||||
func newSecurityCtx(canvasID string) *gin.Context {
|
||||
c, _ := gin.CreateTestContext(httptest.NewRecorder())
|
||||
c.Request = httptest.NewRequest(http.MethodPost, "/api/v1/agents/"+canvasID+"/webhook", nil)
|
||||
return c
|
||||
}
|
||||
|
||||
@@ -61,6 +61,12 @@ func (f *fakeCanvasLoader) RunAgentWithWebhook(_ context.Context, _, _ string, _
|
||||
// makeWebhookCanvas builds a minimal canvas with a Begin component
|
||||
// whose params.mode == "Webhook" and the supplied params map. The
|
||||
// `params` argument becomes webhook_cfg inside the handler.
|
||||
//
|
||||
// As of PR #14890 the webhook requires a security block — empty
|
||||
// configs are rejected. Tests that don't care about auth inject
|
||||
// an explicit anonymous-opt-in block (auth_type=none +
|
||||
// allow_anonymous=true) so the handler proceeds to the
|
||||
// schema/content-type checks under test.
|
||||
func makeWebhookCanvas(id, userID, mode string, params map[string]any) *entity.UserCanvas {
|
||||
dsl := map[string]any{
|
||||
"components": map[string]any{
|
||||
@@ -74,6 +80,15 @@ func makeWebhookCanvas(id, userID, mode string, params map[string]any) *entity.U
|
||||
},
|
||||
},
|
||||
}
|
||||
if params == nil {
|
||||
params = map[string]any{}
|
||||
}
|
||||
if _, ok := params["security"]; !ok {
|
||||
params["security"] = map[string]any{
|
||||
"auth_type": "none",
|
||||
"allow_anonymous": true,
|
||||
}
|
||||
}
|
||||
for k, v := range params {
|
||||
dsl["components"].(map[string]any)["begin"].(map[string]any)["obj"].(map[string]any)["params"].(map[string]any)[k] = v
|
||||
}
|
||||
|
||||
@@ -40,6 +40,7 @@ type userTokenResolver interface {
|
||||
GetUserByToken(authorization string) (*entity.User, common.ErrorCode, error)
|
||||
GetUserByAPIToken(token string) (*entity.User, common.ErrorCode, error)
|
||||
GetUserByBetaAPIToken(token string) (*entity.User, common.ErrorCode, error)
|
||||
GetAPITokenByBeta(authorization string) (*entity.APIToken, error)
|
||||
}
|
||||
|
||||
// NewAuthHandler create auth handler
|
||||
@@ -81,15 +82,33 @@ func (h *AuthHandler) BetaAuthMiddleware() gin.HandlerFunc {
|
||||
c.Next()
|
||||
return
|
||||
}
|
||||
// Then try a regular API token (non-beta public bot flow).
|
||||
if u, code, err := h.userService.GetUserByAPIToken(auth); err == nil && code == common.CodeSuccess {
|
||||
c.Set("user", u)
|
||||
c.Set("auth_via_api_token", true)
|
||||
c.Next()
|
||||
return
|
||||
}
|
||||
// Fall back to beta API token (public bot access).
|
||||
// Fall back to beta API token (public bot access). The
|
||||
// middleware also looks up the APIToken directly so the
|
||||
// downstream handler can read its DialogID (the real
|
||||
// agent_id) without re-parsing the Authorization header.
|
||||
// Mirrors the python
|
||||
// `APIToken.query(beta=token).dialog_id` lookup in
|
||||
// bot_api.py:agent_bot_logs.
|
||||
if u, code, err := h.userService.GetUserByBetaAPIToken(auth); err == nil && code == common.CodeSuccess {
|
||||
c.Set("user", u)
|
||||
if tok, terr := h.userService.GetAPITokenByBeta(auth); terr == nil && tok != nil && tok.DialogID != nil {
|
||||
// tok.DialogID is *string (nullable in the schema), but
|
||||
// downstream handlers (GetAgentbotLogs, GetAgentLogs)
|
||||
// read "agent_id" with agentID.(string) — they cannot
|
||||
// type-assert a *string. Dereference and gate on nil so a
|
||||
// row with a NULL dialog_id still surfaces the
|
||||
// "not bound" sentinel rather than silently leaking the
|
||||
// pointer (which would later fail the string assertion).
|
||||
c.Set("agent_id", *tok.DialogID)
|
||||
c.Set("api_token", tok)
|
||||
}
|
||||
c.Next()
|
||||
return
|
||||
}
|
||||
|
||||
@@ -18,11 +18,14 @@ package handler
|
||||
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
|
||||
"github.com/gin-gonic/gin"
|
||||
|
||||
"ragflow/internal/agent/canvas"
|
||||
"ragflow/internal/common"
|
||||
"ragflow/internal/engine/redis"
|
||||
"ragflow/internal/service"
|
||||
)
|
||||
|
||||
@@ -258,3 +261,61 @@ func (h *BotHandler) ChatbotCompletion(c *gin.Context) {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// GetAgentbotLogs GET /api/v1/agentbots/<shared_id>/logs/<message_id>
|
||||
//
|
||||
// Beta-token sibling of GetAgentLogs. The shared/embedded chat
|
||||
// page's "Thinking" button hits this endpoint because the share
|
||||
// flow authenticates with a beta APIToken (no session JWT) and
|
||||
// the regular /api/v1/agents/<id>/logs/<msg> requires @login_required.
|
||||
// Mirrors python bot_api.py:agent_bot_logs (PR #15238).
|
||||
//
|
||||
// The <shared_id> path segment is the value the client passed in
|
||||
// the URL (typically the beta token in the share flow); the real
|
||||
// agent_id used to build the Redis key
|
||||
// (`<agent_id>-<message_id>-logs`) is read from the APIToken
|
||||
// looked up by the beta middleware and stashed in the gin
|
||||
// context as "agent_id". The endpoint never trusts the URL
|
||||
// segment for the data lookup — using the middleware-resolved
|
||||
// agent_id prevents a probe that swaps a victim's shared_id to
|
||||
// read another agent's logs.
|
||||
func (h *BotHandler) GetAgentbotLogs(c *gin.Context) {
|
||||
if _, code, msg := GetUser(c); code != common.CodeSuccess {
|
||||
jsonError(c, code, msg)
|
||||
return
|
||||
}
|
||||
agentID, _ := c.Get("agent_id")
|
||||
agentIDStr, _ := agentID.(string)
|
||||
if agentIDStr == "" {
|
||||
jsonError(c, common.CodeDataError, "API token is not bound to an agent.")
|
||||
return
|
||||
}
|
||||
messageID := c.Param("message_id")
|
||||
if messageID == "" {
|
||||
jsonError(c, common.CodeArgumentError, "message_id is required")
|
||||
return
|
||||
}
|
||||
key := fmt.Sprintf("%s-%s-logs", agentIDStr, messageID)
|
||||
payload, rerr := redis.Get().Get(key)
|
||||
// Surface Redis / decode failures instead of silently returning
|
||||
// `{code: 0, data: {}}` — the previous form made the endpoint
|
||||
// indistinguishable from "logs not yet written", which masked
|
||||
// real outages and corrupted payloads from operators (PR review
|
||||
// round 5, Major #6).
|
||||
if rerr != nil {
|
||||
jsonError(c, common.CodeServerError, "failed to read agent logs")
|
||||
return
|
||||
}
|
||||
data := map[string]interface{}{}
|
||||
if payload != "" {
|
||||
if uerr := json.Unmarshal([]byte(payload), &data); uerr != nil {
|
||||
jsonError(c, common.CodeServerError, "failed to decode agent logs")
|
||||
return
|
||||
}
|
||||
}
|
||||
c.JSON(200, gin.H{
|
||||
"code": common.CodeSuccess,
|
||||
"data": data,
|
||||
"message": "success",
|
||||
})
|
||||
}
|
||||
|
||||
@@ -800,6 +800,7 @@ type stubUserTokenResolver struct {
|
||||
getUserByTokenFn func(authorization string) (*entity.User, common.ErrorCode, error)
|
||||
getUserByAPITokenFn func(token string) (*entity.User, common.ErrorCode, error)
|
||||
getUserByBetaAPITokenFn func(token string) (*entity.User, common.ErrorCode, error)
|
||||
getAPITokenByBetaFn func(authorization string) (*entity.APIToken, error)
|
||||
}
|
||||
|
||||
func (s *stubUserTokenResolver) GetUserByToken(authorization string) (*entity.User, common.ErrorCode, error) {
|
||||
@@ -823,6 +824,13 @@ func (s *stubUserTokenResolver) GetUserByBetaAPIToken(token string) (*entity.Use
|
||||
return nil, common.CodeUnauthorized, errors.New("not stubbed")
|
||||
}
|
||||
|
||||
func (s *stubUserTokenResolver) GetAPITokenByBeta(authorization string) (*entity.APIToken, error) {
|
||||
if s.getAPITokenByBetaFn != nil {
|
||||
return s.getAPITokenByBetaFn(authorization)
|
||||
}
|
||||
return nil, errors.New("not stubbed")
|
||||
}
|
||||
|
||||
// TestBotRoutes_NoRegularAuthRequired covers criterion 25. The
|
||||
// /api/v1/chatbots/* and /api/v1/agentbots/* routes are mounted
|
||||
// on apiNoAuth (NOT on the auth-protected v1 tree). This test
|
||||
@@ -1079,3 +1087,65 @@ func TestDownloadAttachment_MissingID(t *testing.T) {
|
||||
func inlineRegisterAgentRoutes(g *gin.RouterGroup, h *AgentHandler) {
|
||||
g.GET("/attachments/:attachment_id/download", h.DownloadAttachment)
|
||||
}
|
||||
|
||||
// TestGetAgentbotLogs_RequiresAgentIDInContext guards PR #15238:
|
||||
// the shared/embedded "Thinking" endpoint requires the beta
|
||||
// middleware to have stashed the APIToken.DialogID as "agent_id"
|
||||
// in the gin context. Without it, the handler cannot build the
|
||||
// Redis key and must return the "API token is not bound to an
|
||||
// agent." error — never read the URL's <shared_id> for the lookup.
|
||||
func TestGetAgentbotLogs_RequiresAgentIDInContext(t *testing.T) {
|
||||
gin.SetMode(gin.TestMode)
|
||||
w := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(w)
|
||||
c.Request = httptest.NewRequest("GET",
|
||||
"/api/v1/agentbots/shared-x/logs/msg-1", nil)
|
||||
c.Set("user", &entity.User{ID: "u1"})
|
||||
|
||||
h := NewBotHandler(nil)
|
||||
h.GetAgentbotLogs(c)
|
||||
|
||||
if w.Code != http.StatusOK {
|
||||
t.Fatalf("status = %d, want 200; body = %s", w.Code, w.Body.String())
|
||||
}
|
||||
var resp struct {
|
||||
Code int `json:"code"`
|
||||
Message string `json:"message"`
|
||||
}
|
||||
_ = json.Unmarshal(w.Body.Bytes(), &resp)
|
||||
if resp.Code != int(common.CodeDataError) {
|
||||
t.Errorf("code = %d, want %d (CodeDataError)", resp.Code, common.CodeDataError)
|
||||
}
|
||||
if !strings.Contains(resp.Message, "not bound") {
|
||||
t.Errorf("message = %q, want it to mention 'not bound'", resp.Message)
|
||||
}
|
||||
}
|
||||
|
||||
// TestGetAgentbotLogs_MissingMessageID asserts the param contract:
|
||||
// message_id is required (used to build the Redis key).
|
||||
func TestGetAgentbotLogs_MissingMessageID(t *testing.T) {
|
||||
gin.SetMode(gin.TestMode)
|
||||
w := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(w)
|
||||
c.Request = httptest.NewRequest("GET",
|
||||
"/api/v1/agentbots/shared-x/logs/", nil)
|
||||
c.Set("user", &entity.User{ID: "u1"})
|
||||
c.Set("agent_id", "agent-real")
|
||||
// Gin's path param extraction returns "" for a missing
|
||||
// segment so the handler must reject with CodeArgumentError.
|
||||
|
||||
h := NewBotHandler(nil)
|
||||
h.GetAgentbotLogs(c)
|
||||
|
||||
var resp struct {
|
||||
Code int `json:"code"`
|
||||
Message string `json:"message"`
|
||||
}
|
||||
_ = json.Unmarshal(w.Body.Bytes(), &resp)
|
||||
if resp.Code != int(common.CodeArgumentError) {
|
||||
t.Errorf("code = %d, want %d", resp.Code, common.CodeArgumentError)
|
||||
}
|
||||
if !strings.Contains(resp.Message, "message_id") {
|
||||
t.Errorf("message = %q, want it to mention 'message_id'", resp.Message)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -59,7 +59,7 @@ type documentServiceIface interface {
|
||||
DeleteDocumentMetadata(docID string, keys []string) error
|
||||
DeleteDocumentAllMetadata(docID string) error
|
||||
GetDocumentMetadataByID(docID string) (map[string]interface{}, error)
|
||||
GetDocumentArtifact(filename string) (*service.ArtifactResponse, error)
|
||||
GetDocumentArtifact(filename, userID string) (*service.ArtifactResponse, error)
|
||||
GetDocumentPreview(docID string) (*service.DocumentPreview, error)
|
||||
UploadLocalDocuments(kb *entity.Knowledgebase, tenantID string, files []*multipart.FileHeader, parentPath string, parserConfigOverride map[string]interface{}) ([]map[string]interface{}, []string)
|
||||
UploadWebDocument(kb *entity.Knowledgebase, tenantID, name, url string) (map[string]interface{}, common.ErrorCode, error)
|
||||
@@ -219,8 +219,13 @@ func (h *DocumentHandler) GetDocumentImage(c *gin.Context) {
|
||||
}
|
||||
|
||||
func (h *DocumentHandler) GetDocumentArtifact(c *gin.Context) {
|
||||
user, code, msg := GetUser(c)
|
||||
if code != common.CodeSuccess {
|
||||
jsonError(c, code, msg)
|
||||
return
|
||||
}
|
||||
filename := c.Param("filename")
|
||||
artifact, err := h.documentService.GetDocumentArtifact(filename)
|
||||
artifact, err := h.documentService.GetDocumentArtifact(filename, user.ID)
|
||||
if err != nil {
|
||||
switch {
|
||||
case errors.Is(err, service.ErrArtifactInvalidFilename),
|
||||
|
||||
@@ -81,7 +81,7 @@ func (f *fakeDocumentService) UploadDocumentInfoByURL(userID, rawURL string) (ma
|
||||
return nil, common.CodeSuccess, nil
|
||||
}
|
||||
|
||||
func (f *fakeDocumentService) GetDocumentArtifact(filename string) (*service.ArtifactResponse, error) {
|
||||
func (f *fakeDocumentService) GetDocumentArtifact(filename, _ string) (*service.ArtifactResponse, error) {
|
||||
if filename == "error.txt" {
|
||||
return nil, service.ErrArtifactNotFound
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user