Close redaction gaps + honor the poll-attempt cap
Build and Package / Build Binaries (pull_request) Canceled after 0s

- redact.go: scan the interior of string values for secret-keyed
  key:value / key=value assignments, so a POSTGRES_PASSWORD embedded in a
  custom app's compose YAML (returned by get_app_config as one blob) is
  masked instead of reaching the model context. Non-secret lines and
  ordinary prose are left byte-for-byte unchanged.
- registry.go: run the raw pool.query response through RedactJSON before
  embedding it in handleQueryPools' parse-error string; handler errors are
  returned unredacted, bypassing the normal choke point.
- poller.go/types.go: honor PollerConfig.MaxPollAttempts. It was declared
  ("0 = unlimited") but never read, so it read as a safety cap that wasn't
  enforced. A task now fails once it exhausts the cap; 0 stays unlimited,
  so default behavior is unchanged.

Flips the redact_gaps compose-blob test from asserting the gap to
asserting it is closed.
This commit is contained in:
flan
2026-07-19 19:38:40 +00:00
parent 26d5eda1c7
commit a775b23283
5 changed files with 86 additions and 15 deletions
+24
View File
@@ -54,11 +54,32 @@ func (p *Poller) pollAllTasks() {
}
}
// attemptsExhausted records one poll against the task and reports whether the
// configured MaxPollAttempts cap has been reached. A cap of 0 means unlimited (the
// default), so this is a no-op unless an operator sets a limit; when the cap is hit
// the task is failed so GetActive stops returning it and the poll loop lets go.
func (p *Poller) attemptsExhausted(task *Task) bool {
if p.config.MaxPollAttempts <= 0 {
return false
}
task.Attempts++
if task.Attempts < p.config.MaxPollAttempts {
return false
}
task.Status = TaskStatusFailed
task.StatusMessage = fmt.Sprintf("gave up polling after %d attempts without reaching a terminal state", task.Attempts)
p.store.Update(task)
return true
}
// pollJobTask polls a job-based task using core.get_jobs
func (p *Poller) pollJobTask(task *Task) {
if task.JobID == nil {
return
}
if p.attemptsExhausted(task) {
return
}
// Query job status
result, err := p.client.Call("core.get_jobs", []interface{}{
@@ -86,6 +107,9 @@ func (p *Poller) pollStatusTask(task *Task) {
if task.StatusMethod == "" {
return
}
if p.attemptsExhausted(task) {
return
}
// Call the status method
result, err := p.client.Call(task.StatusMethod)
+1
View File
@@ -35,6 +35,7 @@ type Task struct {
// Internal fields (not exposed in JSON)
OperationType OperationType `json:"-"`
Attempts int `json:"-"` // Poll attempts so far (for MaxPollAttempts)
JobID *int `json:"-"` // For job-based ops
StatusMethod string `json:"-"` // For status-based ops
ToolName string `json:"-"`
+40 -6
View File
@@ -3,6 +3,7 @@ package tools
import (
"bytes"
"encoding/json"
"regexp"
"strings"
)
@@ -19,12 +20,13 @@ import (
// legitimate reason for a credential to reach the model, and "the operator
// remembered to field-filter" is not a control.
//
// KNOWN LIMIT, stated plainly: this masks by KEY NAME (plus the name/value pair
// shape below). A secret embedded inside an opaque string -- e.g. a custom app's
// docker-compose YAML returned as one blob, with `POSTGRES_PASSWORD: hunter2`
// inside it -- is NOT caught, because the key holding the blob isn't credential-
// shaped and we will not regex the interior of arbitrary strings. get_app_config on
// a custom/compose app is therefore not fully safe; prefer query_apps for those.
// Redaction works on three shapes: by KEY NAME, by the name/value pair shape (see
// envPairSecret below), and -- for a secret embedded inside an opaque string, e.g. a
// custom app's docker-compose YAML returned as one blob with `POSTGRES_PASSWORD:
// hunter2` inside it -- by scanning the interior of string values line by line (see
// redactStringBlob). The blob scan only touches lines shaped like `key: value` or
// `key=value` whose key is credential-shaped, so ordinary prose values are left
// byte-for-byte intact. get_app_config on a custom/compose app is covered by this.
const redactedMarker = "***REDACTED***"
@@ -114,11 +116,43 @@ func redactValue(v interface{}) interface{} {
out[i] = redactValue(val)
}
return out
case string:
return redactStringBlob(node)
default:
return v
}
}
// blobAssignLine matches a single "key: value" or "key=value" assignment as found
// inside a compose / env / YAML blob returned as one opaque string. The optional
// leading "- " covers compose's list-form environment (`- POSTGRES_PASSWORD=...`).
var blobAssignLine = regexp.MustCompile(`^(\s*-?\s*)([A-Za-z0-9_.\-]+)(\s*[:=]\s*)(.+?)(\s*)$`)
// redactStringBlob masks secret-keyed assignments embedded inside a string value --
// the docker-compose YAML that get_app_config hands back as one field is the case
// that matters. A string with no secret-shaped assignment line is returned exactly
// as given, so ordinary short values and prose are never mangled.
func redactStringBlob(s string) string {
if !strings.ContainsAny(s, ":=") {
return s
}
lines := strings.Split(s, "\n")
changed := false
for i, line := range lines {
m := blobAssignLine.FindStringSubmatch(line)
if m == nil || !looksSecret(m[2]) {
continue
}
// Preserve indent (m[1]), key (m[2]), and separator (m[3]); drop the value.
lines[i] = m[1] + m[2] + m[3] + redactedMarker
changed = true
}
if !changed {
return s
}
return strings.Join(lines, "\n")
}
// RedactJSON masks credential-looking fields in a JSON document. Input that is
// not valid JSON is returned unchanged -- redaction must never destroy a
// response it does not understand.
+20 -8
View File
@@ -75,13 +75,25 @@ func TestRedactJSONPreservesLargeIntegers(t *testing.T) {
}
}
// Honest documentation of the remaining gap: a secret inside an opaque string blob
// (a custom app's compose YAML) is NOT caught. This test asserts the CURRENT
// behaviour so nobody mistakes it for safety -- if someone later fixes it, this
// test fails loudly and should be updated.
func TestRedactJSONDoesNotCatchSecretsInsideStringBlobs(t *testing.T) {
got := RedactJSON(`{"custom_compose_config_string": "services:\n db:\n environment:\n POSTGRES_PASSWORD: hunter2\n"}`)
if !strings.Contains(got, "hunter2") {
t.Skip("string-blob redaction now implemented -- update this test and the README's known-limit note")
// Previously the audit's honestly-documented gap: a secret inside an opaque string
// blob (a custom app's compose YAML) sailed through because redaction masked by key
// name only. redactStringBlob now scans the interior of string values, so this is
// covered -- a POSTGRES_PASSWORD embedded in a compose blob must be masked, while a
// non-secret line in the same blob survives.
func TestRedactJSONMasksSecretsInsideStringBlobs(t *testing.T) {
got := RedactJSON(`{"custom_compose_config_string": "services:\n db:\n environment:\n POSTGRES_PASSWORD: hunter2\n TZ: America/New_York\n ports:\n - 5432:5432\n"}`)
if strings.Contains(got, "hunter2") {
t.Errorf("secret inside a compose-YAML string blob survived:\n%s", got)
}
// The compose-list dotenv form (- KEY=value) must also be caught.
got2 := RedactJSON(`{"env_blob": "FOO=bar\nAPI_TOKEN=sk_live_deadbeef\n"}`)
if strings.Contains(got2, "sk_live_deadbeef") {
t.Errorf("secret inside a KEY=value env blob survived:\n%s", got2)
}
// Non-secret lines, and structural YAML, must be left intact.
for _, keep := range []string{"America/New_York", "services:", "5432:5432", "bar"} {
if !strings.Contains(got+got2, keep) {
t.Errorf("blob scan destroyed a non-secret line %q:\n%s\n%s", keep, got, got2)
}
}
}
+1 -1
View File
@@ -2024,7 +2024,7 @@ func handleQueryPools(client *truenas.Client, args map[string]interface{}) (stri
var pools []map[string]interface{}
if err := json.Unmarshal(result, &pools); err != nil {
return "", fmt.Errorf("failed to parse pools (raw response: %s): %w", string(result), err)
return "", fmt.Errorf("failed to parse pools (raw response: %s): %w", RedactJSON(string(result)), err)
}
formatted, err := json.MarshalIndent(pools, "", " ")