Fix non-root storage hardening for Helm startup

This commit is contained in:
rcourtman 2026-04-16 21:25:06 +01:00
parent 0db5d2e4c3
commit 9855973e5b
12 changed files with 346 additions and 23 deletions

View file

@ -440,6 +440,15 @@ once and resolve only the fixed `.license-key`, `license.enc`, and
filesystem read, write, rename, stat, or delete. Future licensing persistence
changes must not bypass that resolver with raw `filepath.Join(configDir, ...)`
joins or introduce caller-controlled persistence filenames.
That same local-persistence boundary also owns writable-but-not-owned runtime
storage semantics for commercial state. `pkg/licensing/persistence.go` and
`pkg/licensing/conversion_store.go` may harden directories they own to `0700`,
but they must not assume they can chmod the root of a writable Kubernetes or
container-mounted runtime data directory. When the mount root is writable but
not owned by the Pulse process, canonical persistence must keep file-level
secrets hardened at `0600`, fail closed on world-writable storage roots, and
continue operating instead of crashing just because the mount root itself
cannot be chmod-ed.
Hosted entitlement-source loading follows the same rule: `DatabaseSource` must
normalize persisted Cloud/MSP plan aliases and legacy limit keys before runtime
evaluation, but it must not fabricate a canonical `plan_version` from bare

View file

@ -73,16 +73,17 @@ server-side update execution surfaces.
51. `scripts/trigger-release-dry-run.sh`
52. `scripts/trigger-release.sh`
53. `scripts/toggle-mock.sh`
54. `tests/integration/playwright.config.ts`
55. `tests/integration/QUICK_START.md`
56. `tests/integration/README.md`
57. `tests/integration/scripts/bootstrap-hosted-mobile-onboarding.mjs`
58. `tests/integration/scripts/hosted-mobile-token-runtime.mjs`
59. `tests/integration/scripts/hosted-tenant-runtime.mjs`
60. `tests/integration/scripts/managed-dev-runtime.mjs`
61. `tests/integration/scripts/relay-mobile-token-helper.go`
62. `tests/integration/tests/helpers.ts`
63. `tests/integration/tests/runtime-defaults.ts`
54. `deploy/helm/pulse/`
55. `tests/integration/playwright.config.ts`
56. `tests/integration/QUICK_START.md`
57. `tests/integration/README.md`
58. `tests/integration/scripts/bootstrap-hosted-mobile-onboarding.mjs`
59. `tests/integration/scripts/hosted-mobile-token-runtime.mjs`
60. `tests/integration/scripts/hosted-tenant-runtime.mjs`
61. `tests/integration/scripts/managed-dev-runtime.mjs`
62. `tests/integration/scripts/relay-mobile-token-helper.go`
63. `tests/integration/tests/helpers.ts`
64. `tests/integration/tests/runtime-defaults.ts`
## Shared Boundaries
@ -127,6 +128,10 @@ server-side update execution surfaces.
unowned release-cut switch: changing the version string for a new RC or
stable cut belongs to this subsystem and its release-promotion proof path.
7. Preserve release-matched installer and Helm operator documentation links through `scripts/install.sh`, `.github/workflows/helm-pages.yml`, `.github/workflows/publish-helm-chart.yml`, and the chart metadata itself so deployment guidance and packaged chart metadata do not drift back to branch-tip `main` docs when a release line or promoted tag already exists.
The same governed Helm boundary also owns `deploy/helm/pulse/` itself:
chart metadata, default values, templates, and generated chart docs must
stay on the validated release line rather than mutating `main` or packaging
from whatever branch GitHub happened to check out.
8. Add or change operator-facing hosted tenant runtime canary rollout, batch runtime contract reconciliation, canonical hosted route/public URL generation, or control-plane runtime-registry reconciliation through `cmd/pulse-control-plane/main.go`, `internal/cloudcp/docker/manager.go`, `internal/cloudcp/docker/labels.go`, and `internal/cloudcp/tenant_runtime_rollout.go`
9. Add or change the canonical hosted staging smoke operator path through `scripts/run_hosted_staging_smoke.sh`, `tests/integration/scripts/bootstrap-hosted-mobile-onboarding.mjs`, `tests/integration/scripts/hosted-mobile-token-runtime.mjs`, `tests/integration/scripts/hosted-tenant-runtime.mjs`, and `tests/integration/scripts/relay-mobile-token-helper.go`
@ -174,6 +179,11 @@ non-shell installer/update scripts explicit inside the current self-hosted
release-confidence lane instead of leaving them as implied behavior around the
core runtime.
That same release-confidence lane now also owns the shipped Helm chart path,
so release automation, packaged chart metadata, and chart-runtime smoke no
longer depend on unowned `deploy/helm/pulse/` files while the governed
release workflows package and publish those artifacts.
`internal/updates/` is the live deployment and upgrade planner. It owns
deployment-type detection, update-plan generation, adapter selection, server
update sequencing, and rollback-aware update state for supported Pulse

View file

@ -2400,6 +2400,7 @@
"lane": "L1",
"contract": "docs/release-control/v6/internal/subsystems/deployment-installability.md",
"owned_prefixes": [
"deploy/helm/pulse/",
"docs/releases/",
"internal/updates/"
],
@ -2504,6 +2505,19 @@
"internal/api/updates_test.go"
]
},
{
"id": "helm-chart-release-runtime",
"label": "Helm chart release packaging proof",
"match_prefixes": [
"deploy/helm/pulse/"
],
"match_files": [],
"allow_same_subsystem_tests": false,
"test_prefixes": [],
"exact_files": [
"scripts/release_control/subsystem_lookup_test.py"
]
},
{
"id": "release-promotion-metadata-runtime",
"label": "release promotion and artifact dispatch proof",
@ -4248,6 +4262,8 @@
"internal/api/security.go",
"internal/api/security_tokens.go",
"internal/api/system_settings.go",
"internal/crypto/crypto.go",
"internal/securityutil/secure_storage_dir.go",
"internal/telemetry/telemetry.go",
"scripts/telemetry_adoption_report.py",
"SECURITY.md"
@ -4351,6 +4367,21 @@
"frontend-modern/src/utils/__tests__/apiTokenPresentation.test.ts"
]
},
{
"id": "storage-directory-security",
"label": "storage directory hardening proof",
"match_prefixes": [],
"match_files": [
"internal/crypto/crypto.go",
"internal/securityutil/secure_storage_dir.go"
],
"allow_same_subsystem_tests": false,
"test_prefixes": [],
"exact_files": [
"internal/crypto/crypto_test.go",
"internal/securityutil/secure_storage_dir_test.go"
]
},
{
"id": "security-api-surface",
"label": "security API surface proof",

View file

@ -49,7 +49,9 @@ visibility, and privacy controls to operators.
23. `internal/api/system_settings.go`
24. `internal/telemetry/telemetry.go`
25. `internal/api/router_routes_auth_security.go`
26. `scripts/telemetry_adoption_report.py`
26. `internal/crypto/crypto.go`
27. `internal/securityutil/secure_storage_dir.go`
28. `scripts/telemetry_adoption_report.py`
## Shared Boundaries
@ -73,6 +75,7 @@ visibility, and privacy controls to operators.
4. Change security/auth/token transport behavior through the shared `frontend-modern/src/api/security.ts`, `frontend-modern/src/components/Settings/APITokenManager.tsx`, `frontend-modern/src/components/Settings/apiTokenManagerModel.ts`, `frontend-modern/src/components/Settings/useAPITokenManagerState.ts`, `internal/api/security.go`, `internal/api/security_tokens.go`, and `internal/api/system_settings.go` boundary.
5. Change security/privacy settings presentation through the shared `frontend-modern/src/components/Settings/GeneralSettingsPanel.tsx`, `frontend-modern/src/components/Settings/SecurityAuthPanel.tsx`, `frontend-modern/src/components/Settings/SecurityOverviewPanel.tsx`, `frontend-modern/src/components/Settings/QuickSecuritySetup.tsx`, `frontend-modern/src/components/Settings/SecurityPostureSummary.tsx`, `frontend-modern/src/components/Settings/SSOProviderTypeIcon.tsx`, `frontend-modern/src/utils/securityAuthPresentation.ts`, `frontend-modern/src/utils/securityScorePresentation.ts`, `frontend-modern/src/utils/auditLogPresentation.ts`, and `frontend-modern/src/utils/auditWebhookPresentation.ts` boundary.
6. Change operator-facing telemetry/adoption reporting through `scripts/telemetry_adoption_report.py` together with the privacy disclosure whenever release-identity interpretation changes.
7. Change data-at-rest encryption-key storage-root hardening semantics through `internal/crypto/crypto.go` and `internal/securityutil/secure_storage_dir.go` together so writable-but-not-owned runtime storage mounts stay supported without weakening file-level secrecy.
## Forbidden Paths
@ -87,6 +90,7 @@ visibility, and privacy controls to operators.
3. Keep shared frontend settings proof routing aligned whenever security/privacy presentation changes.
4. Keep the checked-in telemetry adoption report aligned with the same release-identity rules used by the runtime telemetry payload.
5. Update this contract whenever a new canonical security, token, auth, or privacy surface becomes part of the governed trust boundary.
6. Keep the shared storage-directory hardening helper and the crypto manager aligned whenever runtime data-root ownership assumptions change.
## Current State
@ -216,6 +220,14 @@ HTTP, but private authenticated runtimes that are only missing optional
hardening controls such as HTTPS on localhost or an API token must route that
guidance through the governed Security Overview posture surfaces instead of
covering the primary app chrome with a persistent warning.
That same governed trust boundary now also owns the runtime contract for
storage-root hardening of at-rest secrets: `internal/crypto/crypto.go` and the
shared `internal/securityutil/secure_storage_dir.go` helper may attempt to
harden storage directories when Pulse owns them, but they must not assume the
process owns the mount root of a writable Kubernetes or container volume.
Mounted storage roots that are writable but not chmod-able must still support
secure startup, while sensitive leaf files such as `.encryption.key` remain
file-hardened at `0600` and world-writable storage roots still fail closed.
That same Security Overview surface must stay action-oriented once those
low-risk states are demoted out of the global banner:
`frontend-modern/src/components/Settings/SecurityOverviewPanel.tsx` and

View file

@ -43,10 +43,7 @@ var errInvalidKeyMaterial = errors.New("invalid encryption key material")
var errUnsafeKeyPath = errors.New("unsafe encryption key path")
func ensureOwnerOnlyDir(dir string) error {
if err := os.MkdirAll(dir, encryptionKeyDirPerm); err != nil {
return err
}
return os.Chmod(dir, encryptionKeyDirPerm)
return securityutil.EnsureSecureStorageDir(dir, encryptionKeyDirPerm)
}
func decodeEncryptionKey(data []byte) ([]byte, error) {

View file

@ -0,0 +1,55 @@
package securityutil
import (
"errors"
"fmt"
"os"
"syscall"
)
var (
secureStorageDirMkdirAllFn = os.MkdirAll
secureStorageDirChmodFn = os.Chmod
secureStorageDirLstatFn = os.Lstat
)
// EnsureSecureStorageDir creates or hardens a storage directory to the desired
// permissions when possible. For pre-mounted runtime storage roots such as
// Kubernetes volume mounts, the running process may be able to write inside the
// directory without owning the mount root itself. In that case, permission
// errors from chmod are tolerated as long as the existing directory is not
// world-writable.
func EnsureSecureStorageDir(dir string, perm os.FileMode) error {
if err := secureStorageDirMkdirAllFn(dir, perm); err != nil {
return err
}
info, err := secureStorageDirLstatFn(dir)
if err != nil {
return err
}
if info.Mode()&os.ModeSymlink != 0 {
return fmt.Errorf("refusing symlink directory path %q", dir)
}
if !info.IsDir() {
return fmt.Errorf("non-directory path %q", dir)
}
if err := secureStorageDirChmodFn(dir, perm); err != nil {
if !isStorageDirPermissionError(err) {
return err
}
if info.Mode().Perm()&0o002 != 0 {
return fmt.Errorf("cannot leave world-writable storage directory %q unhardened: %w", dir, err)
}
return nil
}
return nil
}
func isStorageDirPermissionError(err error) bool {
return errors.Is(err, os.ErrPermission) ||
errors.Is(err, syscall.EPERM) ||
errors.Is(err, syscall.EACCES)
}

View file

@ -0,0 +1,84 @@
package securityutil
import (
"os"
"testing"
"time"
)
type stubFileInfo struct {
mode os.FileMode
}
func (s stubFileInfo) Name() string { return "stub" }
func (s stubFileInfo) Size() int64 { return 0 }
func (s stubFileInfo) Mode() os.FileMode { return s.mode }
func (s stubFileInfo) ModTime() time.Time { return time.Time{} }
func (s stubFileInfo) IsDir() bool { return s.mode.IsDir() }
func (s stubFileInfo) Sys() any { return nil }
func resetSecureStorageDirFns() {
secureStorageDirMkdirAllFn = os.MkdirAll
secureStorageDirChmodFn = os.Chmod
secureStorageDirLstatFn = os.Lstat
}
func TestEnsureSecureStorageDir_CreatesOwnerOnlyDirectory(t *testing.T) {
t.Cleanup(resetSecureStorageDirFns)
root := t.TempDir()
target := root + "/secure"
if err := EnsureSecureStorageDir(target, 0o700); err != nil {
t.Fatalf("EnsureSecureStorageDir() error: %v", err)
}
info, err := os.Stat(target)
if err != nil {
t.Fatalf("stat target: %v", err)
}
if got := info.Mode().Perm(); got != 0o700 {
t.Fatalf("permissions = %o, want 700", got)
}
}
func TestEnsureSecureStorageDir_AllowsExistingCompatibleDirWhenChmodNotPermitted(t *testing.T) {
t.Cleanup(resetSecureStorageDirFns)
secureStorageDirMkdirAllFn = func(string, os.FileMode) error { return nil }
secureStorageDirLstatFn = func(string) (os.FileInfo, error) {
return stubFileInfo{mode: os.ModeDir | 0o775}, nil
}
secureStorageDirChmodFn = func(string, os.FileMode) error { return os.ErrPermission }
if err := EnsureSecureStorageDir("/data", 0o700); err != nil {
t.Fatalf("EnsureSecureStorageDir() error: %v", err)
}
}
func TestEnsureSecureStorageDir_RejectsWorldWritableExistingDirWhenChmodNotPermitted(t *testing.T) {
t.Cleanup(resetSecureStorageDirFns)
secureStorageDirMkdirAllFn = func(string, os.FileMode) error { return nil }
secureStorageDirLstatFn = func(string) (os.FileInfo, error) {
return stubFileInfo{mode: os.ModeDir | 0o777}, nil
}
secureStorageDirChmodFn = func(string, os.FileMode) error { return os.ErrPermission }
if err := EnsureSecureStorageDir("/data", 0o700); err == nil {
t.Fatal("expected world-writable directory error")
}
}
func TestEnsureSecureStorageDir_RejectsSymlinkPaths(t *testing.T) {
t.Cleanup(resetSecureStorageDirFns)
secureStorageDirMkdirAllFn = func(string, os.FileMode) error { return nil }
secureStorageDirLstatFn = func(string) (os.FileInfo, error) {
return stubFileInfo{mode: os.ModeSymlink}, nil
}
if err := EnsureSecureStorageDir("/data", 0o700); err == nil {
t.Fatal("expected symlink directory error")
}
}

View file

@ -10,6 +10,7 @@ import (
"strings"
"time"
"github.com/rcourtman/pulse-go-rewrite/internal/securityutil"
_ "modernc.org/sqlite"
)
@ -44,10 +45,7 @@ type FunnelSummary struct {
}
func ensureOwnerOnlyDir(dir string) error {
if err := os.MkdirAll(dir, privateDirPerm); err != nil {
return err
}
return os.Chmod(dir, privateDirPerm)
return securityutil.EnsureSecureStorageDir(dir, privateDirPerm)
}
func rejectSymlinkOrNonRegular(path string) error {

View file

@ -3,6 +3,7 @@ package licensing
import (
"database/sql"
"fmt"
"os"
"path/filepath"
"strings"
"testing"
@ -103,6 +104,38 @@ func TestConversionStoreQueryPlansUseIndexes(t *testing.T) {
}
}
func TestNewConversionStore_SecuresOwnedDirectoryAndArtifacts(t *testing.T) {
t.Parallel()
baseDir := t.TempDir()
if err := os.Chmod(baseDir, 0o755); err != nil {
t.Fatalf("failed to loosen base dir perms: %v", err)
}
dbPath := filepath.Join(baseDir, "conversion.db")
store, err := NewConversionStore(dbPath)
if err != nil {
t.Fatalf("NewConversionStore() error = %v", err)
}
t.Cleanup(func() { _ = store.Close() })
dirInfo, err := os.Stat(baseDir)
if err != nil {
t.Fatalf("stat base dir: %v", err)
}
if got := dirInfo.Mode().Perm(); got != 0o700 {
t.Fatalf("base dir perms = %o, want 700", got)
}
dbInfo, err := os.Stat(dbPath)
if err != nil {
t.Fatalf("stat db path: %v", err)
}
if got := dbInfo.Mode().Perm(); got != 0o600 {
t.Fatalf("db perms = %o, want 600", got)
}
}
func newConversionPlanTestStore(t *testing.T) *ConversionStore {
t.Helper()

View file

@ -64,10 +64,7 @@ func resolvePersistencePath(configDir string, leaf string) (string, error) {
}
func ensurePersistenceOwnerOnlyDir(dir string) error {
if err := os.MkdirAll(dir, persistencePrivateDirPerm); err != nil {
return err
}
return os.Chmod(dir, persistencePrivateDirPerm)
return securityutil.EnsureSecureStorageDir(dir, persistencePrivateDirPerm)
}
func validatePersistenceRegularFile(path string, info os.FileInfo) error {

View file

@ -289,6 +289,33 @@ func TestPersistenceEnforcesOwnerOnlyPermissions(t *testing.T) {
}
}
func TestNewPersistence_HardensExistingPersistentKeyFileOnLoad(t *testing.T) {
tmpDir := t.TempDir()
keyPath := filepath.Join(tmpDir, PersistentKeyFileName)
if err := os.WriteFile(keyPath, []byte("persistent-key-material"), 0o644); err != nil {
t.Fatalf("failed to write persistent key: %v", err)
}
if err := os.Chmod(keyPath, 0o644); err != nil {
t.Fatalf("failed to loosen persistent key perms: %v", err)
}
p, err := NewPersistence(tmpDir)
if err != nil {
t.Fatalf("NewPersistence() error: %v", err)
}
if p.encryptionKey != "persistent-key-material" {
t.Fatalf("encryptionKey = %q, want persistent key contents", p.encryptionKey)
}
info, err := os.Stat(keyPath)
if err != nil {
t.Fatalf("stat %s: %v", keyPath, err)
}
if got := info.Mode().Perm(); got != 0o600 {
t.Fatalf("%s perms = %o, want 600", filepath.Base(keyPath), got)
}
}
func TestResolvePersistencePathCanonicalizesConfigDir(t *testing.T) {
baseDir := t.TempDir()
inputDir := " " + filepath.Join(baseDir, "nested", "..", "licensing") + string(os.PathSeparator) + ". "

View file

@ -4552,6 +4552,76 @@ class SubsystemLookupTest(unittest.TestCase):
],
)
def test_lookup_paths_assigns_helm_chart_to_deployment_installability(self) -> None:
result = lookup_paths(
[
"deploy/helm/pulse/Chart.yaml",
"deploy/helm/pulse/templates/deployment.yaml",
"deploy/helm/pulse/values.yaml",
]
)
self.assertEqual(result["unowned_runtime_files"], [])
self.assertEqual(
{item["subsystem"] for item in result["impacted_subsystems"]},
{"deployment-installability"},
)
for file_entry in result["files"]:
self.assertEqual(file_entry["classification"], "runtime")
self.assertEqual(
{match["subsystem"] for match in file_entry["matches"]},
{"deployment-installability"},
)
match = file_entry["matches"][0]
self.assertEqual(
match["contract"],
"docs/release-control/v6/internal/subsystems/deployment-installability.md",
)
self.assertEqual(match["lane_context"]["lane_id"], "L1")
self.assertEqual(
match["verification_requirement"]["id"],
"helm-chart-release-runtime",
)
self.assertEqual(
match["verification_requirement"]["exact_files"],
["scripts/release_control/subsystem_lookup_test.py"],
)
def test_lookup_paths_assigns_crypto_storage_hardening_to_security_privacy(self) -> None:
result = lookup_paths(
[
"internal/crypto/crypto.go",
"internal/securityutil/secure_storage_dir.go",
]
)
self.assertEqual(result["unowned_runtime_files"], [])
self.assertEqual(
{item["subsystem"] for item in result["impacted_subsystems"]},
{"security-privacy"},
)
for file_entry in result["files"]:
self.assertEqual(file_entry["classification"], "runtime")
self.assertEqual(
{match["subsystem"] for match in file_entry["matches"]},
{"security-privacy"},
)
match = file_entry["matches"][0]
self.assertEqual(
match["contract"],
"docs/release-control/v6/internal/subsystems/security-privacy.md",
)
self.assertEqual(match["lane_context"]["lane_id"], "L14")
self.assertEqual(
match["verification_requirement"]["id"],
"storage-directory-security",
)
self.assertEqual(
match["verification_requirement"]["exact_files"],
[
"internal/crypto/crypto_test.go",
"internal/securityutil/secure_storage_dir_test.go",
],
)
def test_lookup_paths_assigns_mock_runtime_fixture_authority_to_monitoring(self) -> None:
result = lookup_paths(
[