Skip to content
2 changes: 1 addition & 1 deletion docs/engram-cloud/troubleshooting.md
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ Cloud auth token is runtime config:
export ENGRAM_CLOUD_TOKEN="your-token"
```

The local `~/.engram/cloud.json` stores the server URL. The token is intentionally read from the environment.
The local `~/.engram/cloud.json` stores the server URL and may also store a `token` fallback. `ENGRAM_CLOUD_TOKEN` takes precedence over any token in `cloud.json`; if the env var is unset, Engram falls back to `cloud.json.token`. This fallback is intentional (issue #343) for use cases such as background autosync where exporting the env var on every shell is not practical.

---

Expand Down
24 changes: 24 additions & 0 deletions internal/store/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -483,6 +483,11 @@ func (s *Store) MaxObservationLength() int {
return s.cfg.MaxObservationLength
}

// DataDir returns the configured data directory for the store.
func (s *Store) DataDir() string {
return s.cfg.DataDir
}

// ─── Store ───────────────────────────────────────────────────────────────────

type Store struct {
Expand Down Expand Up @@ -3761,6 +3766,25 @@ func (s *Store) CountPendingNonEnrolledSyncMutations(targetKey string) ([]Pendin
return counts, rows.Err()
}

// CountPendingSyncMutations returns the number of unacknowledged mutations for
// the given target that are currently eligible to sync (global/project-empty or
// enrolled projects).
func (s *Store) CountPendingSyncMutations(targetKey string) (int64, error) {
targetKey = normalizeSyncTargetKey(targetKey)
var count int64
row := s.db.QueryRow(`
SELECT COUNT(*)
FROM sync_mutations sm
LEFT JOIN sync_enrolled_projects sep ON sm.project = sep.project
WHERE sm.target_key = ? AND sm.acked_at IS NULL
AND (sm.project = '' OR sep.project IS NOT NULL)
`, targetKey)
if err := row.Scan(&count); err != nil {
return 0, err
}
return count, nil
}

// SkipAckNonEnrolledMutations acks (marks as skipped) all pending mutations
// that belong to non-enrolled projects, preventing journal bloat. Empty-project
// mutations are never skipped — they always sync regardless of enrollment.
Expand Down
53 changes: 53 additions & 0 deletions internal/store/store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1382,6 +1382,59 @@ func TestSessionObservationsAddPromptImportAndSyncChunks(t *testing.T) {
}
}

func TestStoreDataDir(t *testing.T) {
s := newTestStore(t)
if got := s.DataDir(); got == "" {
t.Fatal("DataDir should not be empty")
}
}

func TestCountPendingSyncMutations(t *testing.T) {
s := newTestStore(t)

count, err := s.CountPendingSyncMutations(DefaultSyncTargetKey)
if err != nil {
t.Fatalf("CountPendingSyncMutations empty: %v", err)
}
if count != 0 {
t.Fatalf("count = %d, want 0", count)
}

if err := s.EnrollProject("engram"); err != nil {
t.Fatalf("enroll: %v", err)
}
if err := s.CreateSession("s1", "engram", "/tmp/engram"); err != nil {
t.Fatalf("create session: %v", err)
}

count, err = s.CountPendingSyncMutations(DefaultSyncTargetKey)
if err != nil {
t.Fatalf("CountPendingSyncMutations after session: %v", err)
}
if count != 1 {
t.Fatalf("count = %d, want 1", count)
}

if _, err := s.AddObservation(AddObservationParams{
SessionID: "s1",
Type: "bugfix",
Title: "one",
Content: "content",
Project: "engram",
Scope: "project",
}); err != nil {
t.Fatalf("add observation: %v", err)
}

count, err = s.CountPendingSyncMutations(DefaultSyncTargetKey)
if err != nil {
t.Fatalf("CountPendingSyncMutations after observation: %v", err)
}
if count != 2 {
t.Fatalf("count = %d, want 2", count)
}
}
Comment on lines +1392 to +1436

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Missing negative-path coverage for the enrollment exclusion filter.

The test only exercises mutations for the enrolled "engram" project; it never creates a mutation for a project that is not enrolled, so the core new gate (sm.project = '' OR sep.project IS NOT NULL) in CountPendingSyncMutations is never verified to actually exclude non-enrolled projects.

As per path instructions, **/*_test.go should "Verify coverage of happy path, error paths, and edge cases."

🧪 Suggested addition
 	count, err = s.CountPendingSyncMutations(DefaultSyncTargetKey)
 	if err != nil {
 		t.Fatalf("CountPendingSyncMutations after observation: %v", err)
 	}
 	if count != 2 {
 		t.Fatalf("count = %d, want 2", count)
 	}
+
+	// A mutation for a non-enrolled project must not be counted.
+	if err := s.CreateSession("s2", "other-project", "/tmp/other"); err != nil {
+		t.Fatalf("create session for unenrolled project: %v", err)
+	}
+	count, err = s.CountPendingSyncMutations(DefaultSyncTargetKey)
+	if err != nil {
+		t.Fatalf("CountPendingSyncMutations after unenrolled mutation: %v", err)
+	}
+	if count != 2 {
+		t.Fatalf("count = %d, want 2 (unenrolled project mutation should be excluded)", count)
+	}
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
func TestCountPendingSyncMutations(t *testing.T) {
s := newTestStore(t)
count, err := s.CountPendingSyncMutations(DefaultSyncTargetKey)
if err != nil {
t.Fatalf("CountPendingSyncMutations empty: %v", err)
}
if count != 0 {
t.Fatalf("count = %d, want 0", count)
}
if err := s.EnrollProject("engram"); err != nil {
t.Fatalf("enroll: %v", err)
}
if err := s.CreateSession("s1", "engram", "/tmp/engram"); err != nil {
t.Fatalf("create session: %v", err)
}
count, err = s.CountPendingSyncMutations(DefaultSyncTargetKey)
if err != nil {
t.Fatalf("CountPendingSyncMutations after session: %v", err)
}
if count != 1 {
t.Fatalf("count = %d, want 1", count)
}
if _, err := s.AddObservation(AddObservationParams{
SessionID: "s1",
Type: "bugfix",
Title: "one",
Content: "content",
Project: "engram",
Scope: "project",
}); err != nil {
t.Fatalf("add observation: %v", err)
}
count, err = s.CountPendingSyncMutations(DefaultSyncTargetKey)
if err != nil {
t.Fatalf("CountPendingSyncMutations after observation: %v", err)
}
if count != 2 {
t.Fatalf("count = %d, want 2", count)
}
}
func TestCountPendingSyncMutations(t *testing.T) {
s := newTestStore(t)
count, err := s.CountPendingSyncMutations(DefaultSyncTargetKey)
if err != nil {
t.Fatalf("CountPendingSyncMutations empty: %v", err)
}
if count != 0 {
t.Fatalf("count = %d, want 0", count)
}
if err := s.EnrollProject("engram"); err != nil {
t.Fatalf("enroll: %v", err)
}
if err := s.CreateSession("s1", "engram", "/tmp/engram"); err != nil {
t.Fatalf("create session: %v", err)
}
count, err = s.CountPendingSyncMutations(DefaultSyncTargetKey)
if err != nil {
t.Fatalf("CountPendingSyncMutations after session: %v", err)
}
if count != 1 {
t.Fatalf("count = %d, want 1", count)
}
if _, err := s.AddObservation(AddObservationParams{
SessionID: "s1",
Type: "bugfix",
Title: "one",
Content: "content",
Project: "engram",
Scope: "project",
}); err != nil {
t.Fatalf("add observation: %v", err)
}
count, err = s.CountPendingSyncMutations(DefaultSyncTargetKey)
if err != nil {
t.Fatalf("CountPendingSyncMutations after observation: %v", err)
}
if count != 2 {
t.Fatalf("count = %d, want 2", count)
}
// A mutation for a non-enrolled project must not be counted.
if err := s.CreateSession("s2", "other-project", "/tmp/other"); err != nil {
t.Fatalf("create session for unenrolled project: %v", err)
}
count, err = s.CountPendingSyncMutations(DefaultSyncTargetKey)
if err != nil {
t.Fatalf("CountPendingSyncMutations after unenrolled mutation: %v", err)
}
if count != 2 {
t.Fatalf("count = %d, want 2 (unenrolled project mutation should be excluded)", count)
}
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/store/store_test.go` around lines 1392 - 1436, Extend
TestCountPendingSyncMutations to create a mutation for a project that is not
enrolled, then verify CountPendingSyncMutations excludes it while still counting
the enrolled “engram” mutations. Keep the existing enrolled-project assertions
intact and use the available session/observation creation helpers to exercise
the enrollment filter.

Source: Path instructions


func TestStoreLocalSyncFoundationEnqueuesCoreMutations(t *testing.T) {
s := newTestStore(t)

Expand Down
156 changes: 156 additions & 0 deletions internal/tui/cloud.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,156 @@
package tui

import (
"encoding/json"
"fmt"
"net/http"
"net/url"
"os"
"path/filepath"
"strings"
"time"

tea "github.com/charmbracelet/bubbletea"
)

const (
cloudConfigFileName = "cloud.json"
cloudHealthPath = "/health"
)

// Token source labels displayed on the Cloud Config screen.
const (
TokenSourceEnv = "set via ENGRAM_CLOUD_TOKEN"
TokenSourceFile = "read from cloud.json"
TokenSourceNone = "not set"
)

type tuiCloudConfig struct {
ServerURL string `json:"server_url"`
Token string `json:"token,omitempty"`
}

func cloudConfigPath(dataDir string) string {
return filepath.Join(dataDir, cloudConfigFileName)
}

func loadCloudConfig(dataDir string) (*tuiCloudConfig, error) {
path := cloudConfigPath(dataDir)
b, err := os.ReadFile(path)
if err != nil {
if os.IsNotExist(err) {
return &tuiCloudConfig{}, nil
}
return nil, err
}
var cc tuiCloudConfig
if err := json.Unmarshal(b, &cc); err != nil {
return nil, err
}
return &cc, nil
}

func saveCloudConfig(dataDir, serverURL string) error {
if err := os.MkdirAll(dataDir, 0o755); err != nil {
return err
}
cc, err := loadCloudConfig(dataDir)
if err != nil {
return err
}
cc.ServerURL = serverURL
b, err := json.MarshalIndent(cc, "", " ")
if err != nil {
return err
}
return os.WriteFile(cloudConfigPath(dataDir), b, 0o644)
}
Comment on lines +53 to +67

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

cloud.json is written world-readable, exposing the stored token.

saveCloudConfig persists the config (which may contain token, per the fallback documented in docs/engram-cloud/troubleshooting.md) via os.WriteFile(..., 0o644), and the directory is created with os.MkdirAll(dataDir, 0o755). On multi-user systems this makes the cloud token readable by any local user. Credential files should use owner-only permissions (0600/0700), matching conventions used by tools like AWS CLI and SSH.

Note: since os.WriteFile does not change permissions of an already-existing file, an existing world-readable cloud.json won't be tightened by this fix alone; a first-run migration or explicit os.Chmod may be needed for pre-existing files.

🔒 Proposed fix to restrict permissions
 func saveCloudConfig(dataDir, serverURL string) error {
-	if err := os.MkdirAll(dataDir, 0o755); err != nil {
+	if err := os.MkdirAll(dataDir, 0o700); err != nil {
 		return err
 	}
 	cc, err := loadCloudConfig(dataDir)
 	if err != nil {
 		return err
 	}
 	cc.ServerURL = serverURL
 	b, err := json.MarshalIndent(cc, "", "  ")
 	if err != nil {
 		return err
 	}
-	return os.WriteFile(cloudConfigPath(dataDir), b, 0o644)
+	return os.WriteFile(cloudConfigPath(dataDir), b, 0o600)
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
func saveCloudConfig(dataDir, serverURL string) error {
if err := os.MkdirAll(dataDir, 0o755); err != nil {
return err
}
cc, err := loadCloudConfig(dataDir)
if err != nil {
return err
}
cc.ServerURL = serverURL
b, err := json.MarshalIndent(cc, "", " ")
if err != nil {
return err
}
return os.WriteFile(cloudConfigPath(dataDir), b, 0o644)
}
func saveCloudConfig(dataDir, serverURL string) error {
if err := os.MkdirAll(dataDir, 0o700); err != nil {
return err
}
cc, err := loadCloudConfig(dataDir)
if err != nil {
return err
}
cc.ServerURL = serverURL
b, err := json.MarshalIndent(cc, "", " ")
if err != nil {
return err
}
return os.WriteFile(cloudConfigPath(dataDir), b, 0o600)
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/tui/cloud.go` around lines 53 - 67, Update saveCloudConfig to create
dataDir with owner-only permissions (0700) and write cloud.json with owner-only
permissions (0600). Also explicitly tighten permissions on an existing
cloudConfigPath before or after writing, so previously world-readable
configuration files containing tokens are migrated to owner-only access.


func tokenSourceMessage(dataDir string) string {
if strings.TrimSpace(os.Getenv("ENGRAM_CLOUD_TOKEN")) != "" {
return TokenSourceEnv
}
cc, err := loadCloudConfig(dataDir)
if err == nil && strings.TrimSpace(cc.Token) != "" {
return TokenSourceFile
}
return TokenSourceNone
}

func effectiveCloudToken(dataDir string) string {
if token := strings.TrimSpace(os.Getenv("ENGRAM_CLOUD_TOKEN")); token != "" {
return token
}
cc, err := loadCloudConfig(dataDir)
if err != nil {
return ""
}
return strings.TrimSpace(cc.Token)
}

// pingCloudTransport can be overridden in tests to avoid real network calls.
var pingCloudTransport http.RoundTripper = http.DefaultTransport

func pingCloudServer(serverURL, token string) tea.Cmd {
return func() tea.Msg {
status, err := pingCloudServerStatus(serverURL, token)
return cloudPingMsg{status: status, err: err}
}
}

func pingCloudServerStatus(serverURL, token string) (string, error) {
validatedURL, err := validateCloudServerURL(serverURL)
if err != nil {
return "unreachable", err
}

req, err := http.NewRequest(http.MethodGet, validatedURL+cloudHealthPath, nil)
if err != nil {
return "unreachable", err
}
if token != "" {
req.Header.Set("Authorization", "Bearer "+token)
}

client := &http.Client{Timeout: 3 * time.Second, Transport: pingCloudTransport}
resp, err := client.Do(req)
if err != nil {
return "unreachable", err
}
defer resp.Body.Close()

switch {
case resp.StatusCode == http.StatusUnauthorized:
return "unauthorized", nil
case resp.StatusCode >= 200 && resp.StatusCode < 300:
return "reachable", nil
default:
return "unreachable", fmt.Errorf("unexpected status %d", resp.StatusCode)
}
}

func validateCloudServerURL(raw string) (string, error) {
trimmed := strings.TrimSpace(raw)
parsed, err := url.ParseRequestURI(trimmed)
if err != nil {
return "", err
}
scheme := strings.ToLower(strings.TrimSpace(parsed.Scheme))
if scheme != "http" && scheme != "https" {
return "", fmt.Errorf("scheme must be http or https")
}
if strings.TrimSpace(parsed.Host) == "" || strings.TrimSpace(parsed.Hostname()) == "" {
return "", fmt.Errorf("host is required")
}
if strings.TrimSpace(parsed.RawQuery) != "" {
return "", fmt.Errorf("query is not allowed")
}
if strings.TrimSpace(parsed.Fragment) != "" {
return "", fmt.Errorf("fragment is not allowed")
}
parsed.RawQuery = ""
parsed.Fragment = ""
return parsed.String(), nil
}
Comment on lines +132 to +154

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Trailing slash in serverURL produces a malformed health-check URL.

If the user configures a server URL with a trailing slash (e.g. https://cloud.example.com/), parsed.Path becomes "/", which is not stripped here. pingCloudServerStatus then concatenates cloudHealthPath directly, producing https://cloud.example.com//health — likely a 404/unreachable result even though the server is up. This isn't covered by the existing TestValidateCloudServerURL cases.

🐛 Proposed fix to normalize trailing slash
 	parsed.RawQuery = ""
 	parsed.Fragment = ""
+	parsed.Path = strings.TrimRight(parsed.Path, "/")
 	return parsed.String(), nil
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
func validateCloudServerURL(raw string) (string, error) {
trimmed := strings.TrimSpace(raw)
parsed, err := url.ParseRequestURI(trimmed)
if err != nil {
return "", err
}
scheme := strings.ToLower(strings.TrimSpace(parsed.Scheme))
if scheme != "http" && scheme != "https" {
return "", fmt.Errorf("scheme must be http or https")
}
if strings.TrimSpace(parsed.Host) == "" || strings.TrimSpace(parsed.Hostname()) == "" {
return "", fmt.Errorf("host is required")
}
if strings.TrimSpace(parsed.RawQuery) != "" {
return "", fmt.Errorf("query is not allowed")
}
if strings.TrimSpace(parsed.Fragment) != "" {
return "", fmt.Errorf("fragment is not allowed")
}
parsed.RawQuery = ""
parsed.Fragment = ""
return parsed.String(), nil
}
func validateCloudServerURL(raw string) (string, error) {
trimmed := strings.TrimSpace(raw)
parsed, err := url.ParseRequestURI(trimmed)
if err != nil {
return "", err
}
scheme := strings.ToLower(strings.TrimSpace(parsed.Scheme))
if scheme != "http" && scheme != "https" {
return "", fmt.Errorf("scheme must be http or https")
}
if strings.TrimSpace(parsed.Host) == "" || strings.TrimSpace(parsed.Hostname()) == "" {
return "", fmt.Errorf("host is required")
}
if strings.TrimSpace(parsed.RawQuery) != "" {
return "", fmt.Errorf("query is not allowed")
}
if strings.TrimSpace(parsed.Fragment) != "" {
return "", fmt.Errorf("fragment is not allowed")
}
parsed.RawQuery = ""
parsed.Fragment = ""
parsed.Path = strings.TrimRight(parsed.Path, "/")
return parsed.String(), nil
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/tui/cloud.go` around lines 132 - 154, Update validateCloudServerURL
to normalize the parsed path by removing a trailing slash before returning the
canonical URL, so a root URL such as https://cloud.example.com/ becomes
https://cloud.example.com. Preserve non-root paths and existing query/fragment
validation, and ensure pingCloudServerStatus no longer receives a URL that
creates a double slash before cloudHealthPath.



Loading