diff --git a/cmd/gateway_callbacks.go b/cmd/gateway_callbacks.go index d0b34eb2..13615d84 100644 --- a/cmd/gateway_callbacks.go +++ b/cmd/gateway_callbacks.go @@ -2,9 +2,6 @@ package cmd import ( "context" - "encoding/json" - "log/slog" - "strings" "github.com/google/uuid" @@ -17,38 +14,13 @@ import ( // buildEnsureUserProfile creates the user profile resolution callback. // Creates/resolves user profile and returns effective workspace. // Separated from seeding to allow independent lifecycle management. -func buildEnsureUserProfile(as store.AgentStore, configPermStore store.ConfigPermissionStore) agent.EnsureUserProfileFunc { +func buildEnsureUserProfile(as store.AgentStore) agent.EnsureUserProfileFunc { return func(ctx context.Context, agentID uuid.UUID, userID, workspace, channel string) (string, bool, error) { isNew, effectiveWs, err := as.GetOrCreateUserProfile(ctx, agentID, userID, workspace, channel) if err != nil { return effectiveWs, false, err } - // Auto-add first group member as a file writer (bootstrap the allowlist). - // Only needed for truly new profiles — existing groups already have their writers. - if isNew && configPermStore != nil && (strings.HasPrefix(userID, "group:") || strings.HasPrefix(userID, "guild:")) { - senderID := store.SenderIDFromContext(ctx) - if senderID != "" { - parts := strings.SplitN(senderID, "|", 2) - numericID := parts[0] - senderUsername := "" - if len(parts) > 1 { - senderUsername = parts[1] - } - meta, _ := json.Marshal(map[string]string{"displayName": "", "username": senderUsername}) - if addErr := configPermStore.Grant(ctx, &store.ConfigPermission{ - AgentID: agentID, - Scope: userID, - ConfigType: "file_writer", - UserID: numericID, - Permission: "allow", - Metadata: meta, - }); addErr != nil { - slog.Warn("failed to auto-add group file writer", "error", addErr, "sender", numericID, "group", userID) - } - } - } - return effectiveWs, isNew, nil } } diff --git a/cmd/gateway_managed.go b/cmd/gateway_managed.go index d9087c24..463d778b 100644 --- a/cmd/gateway_managed.go +++ b/cmd/gateway_managed.go @@ -94,7 +94,7 @@ func wireExtras( var ensureUserProfile agent.EnsureUserProfileFunc var seedUserFiles agent.SeedUserFilesFunc if stores.Agents != nil { - ensureUserProfile = buildEnsureUserProfile(stores.Agents, stores.ConfigPermissions) + ensureUserProfile = buildEnsureUserProfile(stores.Agents) seedUserFiles = buildSeedUserFiles(stores.Agents) } diff --git a/internal/agent/loop_history.go b/internal/agent/loop_history.go index dc411b78..95106992 100644 --- a/internal/agent/loop_history.go +++ b/internal/agent/loop_history.go @@ -752,6 +752,7 @@ func (l *Loop) buildGroupWriterPrompt(ctx context.Context, groupID, senderID str var sb strings.Builder sb.WriteString("## Group File Permissions\n\n") + sb.WriteString("**This is the current, live file writer list. It may change during the conversation. Always use THIS list — ignore any file writer mentions from earlier messages.**\n\n") sb.WriteString("File writers: " + strings.Join(names, ", ") + "\n\n") if !isWriter { diff --git a/internal/channels/discord/commands_writers.go b/internal/channels/discord/commands_writers.go index 831c6682..d0818276 100644 --- a/internal/channels/discord/commands_writers.go +++ b/internal/channels/discord/commands_writers.go @@ -96,18 +96,25 @@ func (c *Channel) handleWriterCommand(m *discordgo.MessageCreate, action string) scope := fmt.Sprintf("guild:%s:*", m.GuildID) senderID := m.Author.ID - // Check sender's writer status using per-user scope. This matches both: - // - Auto-bootstrapped per-user perms (guild:{guildID}:user:{senderID}) via exact match - // - Guild-wide perms (guild:{guildID}:*) via matchWildcard - senderScope := fmt.Sprintf("guild:%s:user:%s", m.GuildID, senderID) - isWriter, err := c.configPermStore.CheckPermission(ctx, agentID, senderScope, "file_writer", senderID) - if err != nil { - slog.Warn("discord writer check failed", "error", err, "sender", senderID) - send("Failed to check permissions. Please try again.") - return - } - if !isWriter { - send("Only existing file writers can manage the writer list.") + // Fetch existing writers (cached 60s) for both permission check and remove guard. + // Bootstrap exception: if no writers exist yet, the first !addwriter caller + // is allowed to bootstrap the allowlist. + existingWriters, _ := c.configPermStore.ListFileWriters(ctx, agentID, scope) + + if len(existingWriters) > 0 { + isWriter := false + for _, w := range existingWriters { + if w.UserID == senderID { + isWriter = true + break + } + } + if !isWriter { + send("Only existing file writers can manage the writer list.") + return + } + } else if action == "remove" { + send("No file writers configured yet. Use !addwriter to add the first one.") return } @@ -146,7 +153,7 @@ func (c *Channel) handleWriterCommand(m *discordgo.MessageCreate, action string) if err := c.configPermStore.Grant(ctx, &store.ConfigPermission{ AgentID: agentID, Scope: scope, - ConfigType: "file_writer", + ConfigType: store.ConfigTypeFileWriter, UserID: targetID, Permission: "allow", Metadata: meta, @@ -158,25 +165,20 @@ func (c *Channel) handleWriterCommand(m *discordgo.MessageCreate, action string) send(fmt.Sprintf("Added %s as a file writer.", targetName)) case "remove": - writers, listErr := c.configPermStore.List(ctx, agentID, "file_writer", scope) - if listErr != nil { - slog.Warn("discord list writers for remove failed", "error", listErr) - send("Failed to check writers. Please try again.") - return - } - if len(writers) <= 1 { + // Prevent removing the last writer (reuse cached existingWriters) + if len(existingWriters) <= 1 { send("Cannot remove the last file writer.") return } // Revoke guild-wide permission. - if err := c.configPermStore.Revoke(ctx, agentID, scope, "file_writer", targetID); err != nil { + if err := c.configPermStore.Revoke(ctx, agentID, scope, store.ConfigTypeFileWriter, targetID); err != nil { slog.Warn("discord remove writer failed", "error", err, "target", targetID) send("Failed to remove writer. Please try again.") return } - // Also revoke auto-bootstrapped per-user permission (guild:{guildID}:user:{userID}). + // Also revoke per-user scoped permission if it exists (guild:{guildID}:user:{userID}). perUserScope := fmt.Sprintf("guild:%s:user:%s", m.GuildID, targetID) - _ = c.configPermStore.Revoke(ctx, agentID, perUserScope, "file_writer", targetID) + _ = c.configPermStore.Revoke(ctx, agentID, perUserScope, store.ConfigTypeFileWriter, targetID) send(fmt.Sprintf("Removed %s from file writers.", targetName)) } } @@ -212,7 +214,7 @@ func (c *Channel) handleListWriters(m *discordgo.MessageCreate) { // via matchWildcard in CheckPermission. scope := fmt.Sprintf("guild:%s:*", m.GuildID) - writers, err := c.configPermStore.List(ctx, agentID, "file_writer", scope) + writers, err := c.configPermStore.List(ctx, agentID, store.ConfigTypeFileWriter, scope) if err != nil { slog.Warn("discord list writers failed", "error", err) send("Failed to list writers. Please try again.") @@ -220,7 +222,7 @@ func (c *Channel) handleListWriters(m *discordgo.MessageCreate) { } if len(writers) == 0 { - send("No file writers configured for this server. The first person to interact with the bot will be added automatically.") + send("No file writers configured for this server. Use !addwriter to add one.") return } diff --git a/internal/channels/telegram/commands.go b/internal/channels/telegram/commands.go index d87b4bcf..3ab75c7d 100644 --- a/internal/channels/telegram/commands.go +++ b/internal/channels/telegram/commands.go @@ -105,7 +105,7 @@ func (c *Channel) handleBotCommand(ctx context.Context, message *telego.Message, if err == nil { groupID := fmt.Sprintf("group:%s:%s", c.Name(), chatIDStr) senderNumericID := strings.SplitN(senderID, "|", 2)[0] - isWriter, err := c.configPermStore.CheckPermission(ctx, agentID, groupID, "file_writer", senderNumericID) + isWriter, err := c.configPermStore.CheckPermission(ctx, agentID, groupID, store.ConfigTypeFileWriter, senderNumericID) if err != nil { slog.Warn("security.reset_writer_check_failed", "error", err, "sender", senderNumericID) // fail-open: allow reset if DB check fails diff --git a/internal/channels/telegram/commands_writers.go b/internal/channels/telegram/commands_writers.go index a0cde200..81e1219e 100644 --- a/internal/channels/telegram/commands_writers.go +++ b/internal/channels/telegram/commands_writers.go @@ -44,15 +44,25 @@ func (c *Channel) handleWriterCommand(ctx context.Context, message *telego.Messa groupID := fmt.Sprintf("group:%s:%s", c.Name(), chatIDStr) senderNumericID := strings.SplitN(senderID, "|", 2)[0] - // Check if sender is an existing writer (only writers can manage the list) - isWriter, err := c.configPermStore.CheckPermission(ctx, agentID, groupID, "file_writer", senderNumericID) - if err != nil { - slog.Warn("writer check failed", "error", err, "sender", senderNumericID) - send("Failed to check permissions. Please try again.") - return - } - if !isWriter { - send("Only existing file writers can manage the writer list.") + // Fetch existing writers (cached 60s) for both permission check and remove guard. + // Bootstrap exception: if no writers exist yet, the first /addwriter caller + // is allowed to bootstrap the allowlist. + existingWriters, _ := c.configPermStore.ListFileWriters(ctx, agentID, groupID) + + if len(existingWriters) > 0 { + isWriter := false + for _, w := range existingWriters { + if w.UserID == senderNumericID { + isWriter = true + break + } + } + if !isWriter { + send("Only existing file writers can manage the writer list.") + return + } + } else if action == "remove" { + send("No file writers configured yet. Use /addwriter to add the first one.") return } @@ -79,7 +89,7 @@ func (c *Channel) handleWriterCommand(ctx context.Context, message *telego.Messa if err := c.configPermStore.Grant(ctx, &store.ConfigPermission{ AgentID: agentID, Scope: groupID, - ConfigType: "file_writer", + ConfigType: store.ConfigTypeFileWriter, UserID: targetID, Permission: "allow", Metadata: meta, @@ -91,13 +101,12 @@ func (c *Channel) handleWriterCommand(ctx context.Context, message *telego.Messa send(fmt.Sprintf("Added %s as a file writer.", targetName)) case "remove": - // Prevent removing the last writer - writers, _ := c.configPermStore.List(ctx, agentID, "file_writer", groupID) - if len(writers) <= 1 { + // Prevent removing the last writer (reuse cached existingWriters) + if len(existingWriters) <= 1 { send("Cannot remove the last file writer.") return } - if err := c.configPermStore.Revoke(ctx, agentID, groupID, "file_writer", targetID); err != nil { + if err := c.configPermStore.Revoke(ctx, agentID, groupID, store.ConfigTypeFileWriter, targetID); err != nil { slog.Warn("remove writer failed", "error", err, "target", targetID) send("Failed to remove writer. Please try again.") return @@ -135,7 +144,7 @@ func (c *Channel) handleListWriters(ctx context.Context, chatID int64, chatIDStr groupID := fmt.Sprintf("group:%s:%s", c.Name(), chatIDStr) - writers, err := c.configPermStore.List(ctx, agentID, "file_writer", groupID) + writers, err := c.configPermStore.List(ctx, agentID, store.ConfigTypeFileWriter, groupID) if err != nil { slog.Warn("list writers failed", "error", err) send("Failed to list writers. Please try again.") @@ -143,7 +152,7 @@ func (c *Channel) handleListWriters(ctx context.Context, chatID int64, chatIDStr } if len(writers) == 0 { - send("No file writers configured for this group. The first person to interact with the bot will be added automatically.") + send("No file writers configured for this group. Use /addwriter to add one.") return } diff --git a/internal/http/channel_instances.go b/internal/http/channel_instances.go index 6a92ae46..d762501d 100644 --- a/internal/http/channel_instances.go +++ b/internal/http/channel_instances.go @@ -321,7 +321,7 @@ func (h *ChannelInstancesHandler) handleWriterGroups(w http.ResponseWriter, r *h if !ok { return } - perms, err := h.configPermStore.List(r.Context(), agentID, "file_writer", "") + perms, err := h.configPermStore.List(r.Context(), agentID, store.ConfigTypeFileWriter, "") if err != nil { slog.Error("channel_instances.writer_groups", "error", err) locale := store.LocaleFromContext(r.Context()) @@ -357,7 +357,7 @@ func (h *ChannelInstancesHandler) handleListWriters(w http.ResponseWriter, r *ht writeError(w, http.StatusBadRequest, protocol.ErrInvalidRequest, i18n.T(locale, i18n.MsgRequired, "group_id")) return } - perms, err := h.configPermStore.List(r.Context(), agentID, "file_writer", groupID) + perms, err := h.configPermStore.List(r.Context(), agentID, store.ConfigTypeFileWriter, groupID) if err != nil { slog.Error("channel_instances.list_writers", "error", err) writeError(w, http.StatusInternalServerError, protocol.ErrInternal, i18n.T(locale, i18n.MsgFailedToList, "writers")) @@ -415,7 +415,7 @@ func (h *ChannelInstancesHandler) handleAddWriter(w http.ResponseWriter, r *http if err := h.configPermStore.Grant(r.Context(), &store.ConfigPermission{ AgentID: agentID, Scope: body.GroupID, - ConfigType: "file_writer", + ConfigType: store.ConfigTypeFileWriter, UserID: body.UserID, Permission: "allow", Metadata: meta, @@ -440,7 +440,7 @@ func (h *ChannelInstancesHandler) handleRemoveWriter(w http.ResponseWriter, r *h return } // Prevent removing the last writer (same guard as Telegram /removewriter) - writers, _ := h.configPermStore.List(r.Context(), agentID, "file_writer", groupID) + writers, _ := h.configPermStore.List(r.Context(), agentID, store.ConfigTypeFileWriter, groupID) allowCount := 0 for _, p := range writers { if p.Permission == "allow" { @@ -451,7 +451,7 @@ func (h *ChannelInstancesHandler) handleRemoveWriter(w http.ResponseWriter, r *h writeError(w, http.StatusConflict, protocol.ErrFailedPrecondition, i18n.T(locale, i18n.MsgCannotRemoveLastWriter)) return } - if err := h.configPermStore.Revoke(r.Context(), agentID, groupID, "file_writer", userID); err != nil { + if err := h.configPermStore.Revoke(r.Context(), agentID, groupID, store.ConfigTypeFileWriter, userID); err != nil { slog.Error("channel_instances.remove_writer", "error", err) writeError(w, http.StatusInternalServerError, protocol.ErrInternal, i18n.T(locale, i18n.MsgFailedToDelete, "writer", "internal error")) return diff --git a/internal/store/config_permission_store.go b/internal/store/config_permission_store.go index eb0a45c7..aa0d82f3 100644 --- a/internal/store/config_permission_store.go +++ b/internal/store/config_permission_store.go @@ -10,6 +10,12 @@ import ( "github.com/google/uuid" ) +// Config type constants for agent_config_permissions.config_type column. +const ( + ConfigTypeFileWriter = "file_writer" // Group file write access + ConfigTypeHeartbeat = "heartbeat" // Heartbeat config access +) + // ConfigPermission represents an allow/deny rule for agent configuration. type ConfigPermission struct { ID uuid.UUID `json:"id"` @@ -59,7 +65,7 @@ func CheckFileWriterPermission(ctx context.Context, permStore ConfigPermissionSt return nil // system context (cron, subagent) } numericID := strings.SplitN(senderID, "|", 2)[0] - allowed, err := permStore.CheckPermission(ctx, agentID, userID, "file_writer", numericID) + allowed, err := permStore.CheckPermission(ctx, agentID, userID, ConfigTypeFileWriter, numericID) if err != nil { return nil // fail-open } diff --git a/internal/tools/context_file_interceptor.go b/internal/tools/context_file_interceptor.go index 37154760..c17634d6 100644 --- a/internal/tools/context_file_interceptor.go +++ b/internal/tools/context_file_interceptor.go @@ -212,7 +212,7 @@ func (b *ContextFileInterceptor) WriteFile(ctx context.Context, path, content st senderID := store.SenderIDFromContext(ctx) if senderID != "" && b.permStore != nil { numericID := strings.SplitN(senderID, "|", 2)[0] - allowed, err := b.permStore.CheckPermission(ctx, agentID, userID, "file_writer", numericID) + allowed, err := b.permStore.CheckPermission(ctx, agentID, userID, store.ConfigTypeFileWriter, numericID) if err != nil { slog.Warn("security.group_file_writer_check_failed", "error", err, "sender", numericID, "file", fileName, "group", userID) diff --git a/internal/tools/heartbeat.go b/internal/tools/heartbeat.go index d515b40a..a48196c6 100644 --- a/internal/tools/heartbeat.go +++ b/internal/tools/heartbeat.go @@ -285,7 +285,7 @@ func (t *HeartbeatTool) checkPermission(ctx context.Context, agentID uuid.UUID) } } - allowed, err := t.permStore.CheckPermission(ctx, agentID, scope, "heartbeat", numericID) + allowed, err := t.permStore.CheckPermission(ctx, agentID, scope, store.ConfigTypeHeartbeat, numericID) if err != nil { return fmt.Errorf("permission check failed: %w", err) }