Repository navigation
Encrypt the bot signing secret at rest, and resolve the remaining pkg/security helpers #868
Description
Security issue. Detail is in private advisory GHSA-xqx8-pcpv-cx2h; this issue tracks the remediation only.
pkg/bot persists a chat-platform signing secret to spore-bot-workspaces in plaintext, while pkg/security.EncryptSecret/DecryptSecret/IsEncrypted — KMS helpers with tests — have no caller anywhere. Confirmed against the live table without reading any secret value: four items carry a 32-character signing_secret, which is the plaintext form and which IsEncrypted would correctly report as not encrypted.
Not inflated: the table has SSE-at-rest enabled, and read access is scoped to one role (SpawnDashboardLambdaRole / SporeSlackWorkspaceAccess) rather than the account. So exposure needs that role or admin. But the same class of secret is handled correctly in the same repo — lambda/dashboard-api reads its Slack signing secret from Secrets Manager — so there are two mechanisms for one credential and this is the weaker one.
- Encrypt on write / decrypt on read in
pkg/bot, withIsEncryptedon the read path so existing plaintext rows survive the transition instead of failing closed. - Re-key the four live secrets in Slack. Encrypting the column does not un-expose a value already written; this is the only step that remediates the existing exposure. (Maintainer — live state.)
- Narrow
SporeSlackWorkspaceAccessto the attributes the dashboard needs, ifsigning_secretis not among them. (Maintainer — live IAM.)
This resolves #811's last item, and inverts it
#811 lists eight unreached pkg/security helpers and says "removing them is safe; whether something should be calling SanitizeForLog/ValidateCommand is a different question." Assessed all eight. The answer differs per helper, and "delete them" is wrong for three:
| helper | verdict |
|---|---|
EncryptSecret, DecryptSecret, IsEncrypted |
wire in — the defect above. These are the ones that were meant to be on a path |
ValidateMountPath |
do not wire in. Permits only /mnt, /data, /scratch, while spawn launch's own defaults are /efs and /fsx. Calling it would reject a default launch — it encodes a policy the product does not follow. Delete it or fix its allowlist; a hand-written allowlist that contradicts the defaults is the rule-4 anti-pattern in a security helper |
ValidateCommand |
do not wire in. A denylist of shell metacharacters, and its one plausible call site — --pre-stop — is meant to be a shell command, so a legitimate hook with a pipe would be rejected. The repo already does this correctly: buildPreStopCommand quotes with ShellQuote and validates with ValidateUsername |
MaskSecret |
no call site needed — cmd/bot.go:508 already redacts for display |
MaskURL, SanitizeForLog, SanitizePath |
no candidate site found. No URL carrying credentials is logged anywhere (checked), and the Windows Administrator password printed by cmd/connect.go is the point of that command, not a leak |
Also worth correcting: #811 describes pkg/security as 197 lines of unreached code. The package is substantially live — ValidateUsername has four callers (lambda/ttl-reaper, pkg/agent, pkg/launcher, pkg/pluginruntime), ValidatePathForReading two, ShellQuote two, ValidateBase64 and NormalizeUsername one each. The 197 lines refer to the eight helpers only, and the package should not be approached as a deletion candidate.
Activity
scttfrdmn commentedon Oct 12, 2026
Backlogged — labelled
backlogand removed from the Security audit 2026-10 milestone, so that milestone reflects active work only. Not closed: the finding is verified and real, just not being worked now.State preserved so picking it up is cheap:
pkg/bot—SigningSecretpersisted tospore-bot-workspaceswithdynamodbav:"signing_secret", no encryption anywhere in the package, whilepkg/security.EncryptSecret/DecryptSecret/IsEncryptedhave no caller.signing_secret. Verified without reading any value (projection scan, presence and length only).SpawnDashboardLambdaRole / SporeSlackWorkspaceAccess. Exposure needs that role or account admin.Nothing else depends on this. The
pkg/securityhalf of the question is already answered in full on #811 (which three helpers to wire in, and the two that must not be —ValidateMountPathcontradicts the product's own mount defaults,ValidateCommandis a denylist the codebase has superseded with quoting), so none of that analysis needs redoing.Whenever it is picked up, the order that matters: re-keying the four secrets in Slack is the only step that remediates the existing exposure. Encrypting the column protects future writes and does nothing about values already written.