Backup/ARTEX
1
0
Fork 0
mirror of https://github.com/BaaRai/ARTEX.git synced 2026-11-15 04:30:55 +08:00
Code Issues Packages Projects Releases Wiki Activity

secrets: keep a masked tool secret only for its saved destination

Browse Source 
As for the MCP env (#213), a placeholder in a custom tool's exec keeps the
stored header, proxy or body only while the url (exact string) and the kind
are unchanged. A changed or emptied url, or a kind change, with a resolved
placeholder is refused (400 on the API and the test endpoint, a tool error
for update_custom_tool) before any write and before the test request is
sent. An explicit value with a new url is allowed.
This commit is contained in:
BaaRai
2026-11-13 21:58:19 +01:00
parent 68976a7680
commit 7899662aee
3 changed files with 101 additions and 11 deletions
Download Patch File Download Diff File Expand all files Collapse all files
server/customtool.go
+17 -9
View File
@@ -74,7 +74,7 @@ func (s *Server) pgCreateCustomTool(w http.ResponseWriter, r *http.Request) {
return
}
var err error
if req.Exec, err = unmaskToolExec(nil, req.Exec); err != nil { // nothing stored: a placeholder is refused
if req.Exec, err = unmaskToolExec("", nil, req.Kind, req.Exec); err != nil { // nothing stored: a placeholder is refused
writeErr(w, 400, err.Error())
return
}
@@ -116,7 +116,7 @@ func (s *Server) pgUpdateCustomTool(w http.ResponseWriter, r *http.Request) {
writeErr(w, 400, "http tools must provide a parameter JSON Schema (cannot be left empty)")
return
}
if req.Exec, err = unmaskToolExec(existing.Exec, req.Exec); err != nil {
if req.Exec, err = unmaskToolExec(existing.Kind, existing.Exec, req.Kind, req.Exec); err != nil {
writeErr(w, 400, err.Error())
return
}
@@ -167,6 +167,7 @@ func (s *Server) pgTestCustomTool(w http.ResponseWriter, r *http.Request) {
writeErr(w, 400, "invalid request body")
return
}
var storedKind string
var stored json.RawMessage
if req.Key != "" {
t, err := pg.GetTool(req.Key)
@@ -175,11 +176,11 @@ func (s *Server) pgTestCustomTool(w http.ResponseWriter, r *http.Request) {
return
}
if t != nil && !t.System {
stored = t.Exec
storedKind, stored = t.Kind, t.Exec
}
}
var err error
if req.Exec, err = unmaskToolExec(stored, req.Exec); err != nil {
if req.Exec, err = unmaskToolExec(storedKind, stored, req.Kind, req.Exec); err != nil {
writeErr(w, 400, err.Error())
return
}
@@ -294,14 +295,16 @@ func maskToolExec(raw json.RawMessage) json.RawMessage {
// unmaskToolExec puts the stored value back for every header, proxy or body sent
// as the placeholder, so a client echoing the list response keeps the stored
// secret; an explicit value replaces it and "" clears it. A placeholder with
// nothing stored to keep (stored nil on create) is refused.
func unmaskToolExec(stored, in json.RawMessage) (json.RawMessage, error) {
// nothing stored to keep (stored nil on create) is refused. As for the MCP env,
// a stored secret is only kept for the destination it was configured for: when
// the url (exact string) or the kind changes, a placeholder is refused.
func unmaskToolExec(storedKind string, stored json.RawMessage, kind string, in json.RawMessage) (json.RawMessage, error) {
var e, old map[string]any
if json.Unmarshal(in, &e) != nil || e == nil {
return in, nil // not an object: stored or refused as before
}
_ = json.Unmarshal(stored, &old)
changed := false
changed, kept := false, false
for _, k := range toolExecFlags {
if _, ok := e[k]; ok {
delete(e, k)
@@ -318,7 +321,7 @@ func unmaskToolExec(stored, in json.RawMessage) (json.RawMessage, error) {
if !ok {
return nil, fmt.Errorf("header %q: masked value but nothing stored to keep", k)
}
headers[k], changed = prev, true
headers[k], changed, kept = prev, true, true
}
for _, k := range []string{"proxy", "body"} {
if s, ok := e[k].(string); !ok || !isMaskedSecret(s) {
@@ -328,7 +331,12 @@ func unmaskToolExec(stored, in json.RawMessage) (json.RawMessage, error) {
if prev == "" {
return nil, fmt.Errorf("%s: masked value but nothing stored to keep", k)
}
e[k], changed = prev, true
e[k], changed, kept = prev, true, true
}
oldURL, _ := old["url"].(string)
newURL, _ := e["url"].(string)
if kept && (oldURL != newURL || storedKind != kind) {
return nil, fmt.Errorf("destination changed (url or kind) while a stored secret was kept: re-enter the masked header/proxy/body or clear it; stored secrets are not sent to a new destination")
}
if !changed {
return in, nil
server/platform_tools.go
+2 -2
View File
@@ -217,7 +217,7 @@ func (s *Server) toolCreateCustomTool() actool.CoreTool {
return actool.Errorf("this key already exists: " + a.Key), nil
}
var err error
if a.Exec, err = unmaskToolExec(nil, a.Exec); err != nil { // nothing stored: a placeholder is refused
if a.Exec, err = unmaskToolExec("", nil, a.Kind, a.Exec); err != nil { // nothing stored: a placeholder is refused
return actool.Errorf(err.Error()), nil
}
if err := s.m.pg.CreateCustomTool(toDBTool(a)); err != nil {
@@ -244,7 +244,7 @@ func (s *Server) toolUpdateCustomTool() actool.CoreTool {
return actool.Errorf("http tools must provide a parameter JSON Schema (cannot be empty)"), nil
}
var err error
if a.Exec, err = unmaskToolExec(existing.Exec, a.Exec); err != nil {
if a.Exec, err = unmaskToolExec(existing.Kind, existing.Exec, a.Kind, a.Exec); err != nil {
return actool.Errorf(err.Error()), nil
}
if err := s.m.pg.UpdateCustomTool(toDBTool(a)); err != nil {
server/tools_secrets_api_test.go
+82
View File
@@ -7,9 +7,91 @@ import (
"net/http"
"net/http/httptest"
"strings"
"sync/atomic"
"testing"
)
// A placeholder keeps the stored secret only for the url and kind it was saved
// with: a changed destination is refused before any write or request.
func TestCustomToolExecDestinationGuard(t *testing.T) {
s, _, request := trafficEvidenceServer(t)
pg := s.m.PG()
var hits atomic.Int32
echo := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
hits.Add(1)
io.WriteString(w, "auth="+r.Header.Get("Authorization"))
}))
t.Cleanup(echo.Close)
t.Cleanup(func() { pg.Exec(`DELETE FROM tools WHERE key LIKE 't_secguard%'`) })
const secret = "Bearer guard-plain-0001"
schema := `{"type":"object","properties":{"q":{"type":"string"}}}`
body := func(kind, url, hdr string) string {
e, _ := json.Marshal(map[string]any{"method": "GET", "url": url, "headers": map[string]string{"Authorization": hdr}})
return `{"key":"t_secguard","kind":"` + kind + `","enabled":true,"schema":` + schema + `,"exec":` + string(e) + `}`
}
storedExec := func() (url, auth string) {
tl, err := pg.GetTool("t_secguard")
if err != nil || tl == nil {
t.Fatalf("GetTool: %v", err)
}
var e struct {
URL string `json:"url"`
Headers map[string]string `json:"headers"`
}
_ = json.Unmarshal(tl.Exec, &e)
return e.URL, e.Headers["Authorization"]
}
if resp := request("POST", "/api/tools/custom", body("http", echo.URL, secret)); resp.Code != 200 {
t.Fatalf("create: HTTP %d %s", resp.Code, resp.Body.String())
}
moved := echo.URL + "/elsewhere"
// 1. Masked header, changed url: refused, nothing written.
if resp := request("PUT", "/api/tools/custom/t_secguard", body("http", moved, "__masked__")); resp.Code != 400 {
t.Fatalf("masked header with a changed url must be refused, got HTTP %d", resp.Code)
}
if url, auth := storedExec(); url != echo.URL || auth != secret {
t.Fatalf("a refused update changed the stored exec: %s %s", url, auth)
}
// An empty url against a stored one is a change too.
if resp := request("PUT", "/api/tools/custom/t_secguard", body("http", "", "__masked__")); resp.Code != 400 {
t.Fatalf("masked header with an emptied url must be refused, got HTTP %d", resp.Code)
}
// 2. Masked header, same url: kept.
if resp := request("PUT", "/api/tools/custom/t_secguard", body("http", echo.URL, "__masked__")); resp.Code != 200 {
t.Fatalf("masked header with the same url: HTTP %d %s", resp.Code, resp.Body.String())
}
if url, auth := storedExec(); url != echo.URL || auth != secret {
t.Fatalf("the stored secret was not kept: %s %s", url, auth)
}
// 3. Test endpoint, masked header, changed url: refused, no request sent.
e, _ := json.Marshal(map[string]any{"method": "GET", "url": moved, "headers": map[string]string{"Authorization": "__masked__"}})
if resp := request("POST", "/api/tools/custom/test", `{"key":"t_secguard","kind":"http","exec":`+string(e)+`}`); resp.Code != 400 {
t.Fatalf("test endpoint with a changed url must be refused, got HTTP %d %s", resp.Code, resp.Body.String())
}
if n := hits.Load(); n != 0 {
t.Fatalf("the echo server received %d request(s); the stored secret left", n)
}
// 4. Kind change while keeping a placeholder: refused.
if resp := request("PUT", "/api/tools/custom/t_secguard", body("command", echo.URL, "__masked__")); resp.Code != 400 {
t.Fatalf("a kind change with a placeholder must be refused, got HTTP %d", resp.Code)
}
if tl, _ := pg.GetTool("t_secguard"); tl == nil || tl.Kind != "http" {
t.Fatalf("a refused kind change was written: %v", tl)
}
// 5. An explicit value with a changed url is allowed.
if resp := request("PUT", "/api/tools/custom/t_secguard", body("http", moved, "Bearer typed-0002")); resp.Code != 200 {
t.Fatalf("explicit header with a changed url: HTTP %d %s", resp.Code, resp.Body.String())
}
if url, auth := storedExec(); url != moved || auth != "Bearer typed-0002" {
t.Fatalf("explicit replace not stored: %s %s", url, auth)
}
}
// A custom tool's exec: headers, proxy and body are masked by GET /api/tools;
// the placeholder keeps the stored value on update and in the test endpoint, a
// create with it is refused, and the executor runs on the clear exec.