diff --git a/appview/knots/knots.go b/appview/knots/knots.go index c127f614..c421db00 100644 --- a/appview/knots/knots.go +++ b/appview/knots/knots.go @@ -551,19 +551,6 @@ func (k *Knots) addMember(w http.ResponseWriter, r *http.Request) { return } - if err = k.Enforcer.AddKnotMember(domain, memberId.DID.String()); err != nil { - l.Error("failed to add member to ACLs", "err", err) - fail() - return - } - committed := false - defer func() { - if committed { - return - } - k.Enforcer.E.LoadPolicy() - }() - rkey := tid.TID() _, err = comatproto.RepoPutRecord(r.Context(), client, &comatproto.RepoPutRecord_Input{ Collection: tangled.KnotMemberNSID, @@ -583,13 +570,6 @@ func (k *Knots) addMember(w http.ResponseWriter, r *http.Request) { return } - if err = k.Enforcer.E.SavePolicy(); err != nil { - l.Error("failed to save ACL policy", "err", err) - fail() - return - } - committed = true - k.Pages.HxRedirect(w, fmt.Sprintf("/settings/knots/%s", domain)) } @@ -655,38 +635,22 @@ func (k *Knots) removeMember(w http.ResponseWriter, r *http.Request) { l.Warn("failed to look up member rkey", "err", err) } - if err = k.Enforcer.RemoveKnotMember(domain, memberId.DID.String()); err != nil { - l.Error("failed to update ACLs", "err", err) + if rkey == "" { + l.Error("no member record found to remove") fail() return } - committed := false - defer func() { - if committed { - return - } - k.Enforcer.E.LoadPolicy() - }() - - if rkey != "" { - _, err = comatproto.RepoDeleteRecord(r.Context(), client, &comatproto.RepoDeleteRecord_Input{ - Collection: tangled.KnotMemberNSID, - Repo: user.Did, - Rkey: rkey, - }) - if err != nil { - l.Error("failed to delete record from PDS", "err", err) - k.Pages.Notice(w, noticeId, "Failed to delete record from PDS, try again later.") - return - } - } - if err = k.Enforcer.E.SavePolicy(); err != nil { - l.Error("failed to save ACLs", "err", err) - fail() + _, err = comatproto.RepoDeleteRecord(r.Context(), client, &comatproto.RepoDeleteRecord_Input{ + Collection: tangled.KnotMemberNSID, + Repo: user.Did, + Rkey: rkey, + }) + if err != nil { + l.Error("failed to delete record from PDS", "err", err) + k.Pages.Notice(w, noticeId, "Failed to delete record from PDS, try again later.") return } - committed = true k.Pages.HxRefresh(w) } diff --git a/appview/spindles/spindles.go b/appview/spindles/spindles.go index 4d96f796..42e70126 100644 --- a/appview/spindles/spindles.go +++ b/appview/spindles/spindles.go @@ -489,37 +489,8 @@ func (s *Spindles) addMember(w http.ResponseWriter, r *http.Request) { return } - tx, err := s.Db.Begin() - if err != nil { - l.Error("failed to start txn", "err", err) - fail() - return - } - defer func() { - tx.Rollback() - s.Enforcer.E.LoadPolicy() - }() - rkey := tid.TID() - // add member to db - if err = db.AddSpindleMember(tx, models.SpindleMember{ - Did: syntax.DID(user.Did), - Rkey: rkey, - Instance: instance, - Subject: memberId.DID, - }); err != nil { - l.Error("failed to add spindle member", "err", err) - fail() - return - } - - if err = s.Enforcer.AddSpindleMember(instance, memberId.DID.String()); err != nil { - l.Error("failed to add member to ACLs") - fail() - return - } - _, err = comatproto.RepoPutRecord(r.Context(), client, &comatproto.RepoPutRecord_Input{ Collection: tangled.SpindleMemberNSID, Repo: user.Did, @@ -538,18 +509,6 @@ func (s *Spindles) addMember(w http.ResponseWriter, r *http.Request) { return } - if err = tx.Commit(); err != nil { - l.Error("failed to commit txn", "err", err) - fail() - return - } - - if err = s.Enforcer.E.SavePolicy(); err != nil { - l.Error("failed to add member to ACLs", "err", err) - fail() - return - } - // success s.Pages.HxRedirect(w, fmt.Sprintf("/settings/spindles/%s", instance)) } @@ -607,18 +566,6 @@ func (s *Spindles) removeMember(w http.ResponseWriter, r *http.Request) { return } - tx, err := s.Db.Begin() - if err != nil { - l.Error("failed to start txn", "err", err) - fail() - return - } - defer func() { - tx.Rollback() - s.Enforcer.E.LoadPolicy() - }() - - // get the record from the DB first: members, err := db.GetSpindleMembers( s.Db, orm.FilterEq("did", user.Did), @@ -631,25 +578,6 @@ func (s *Spindles) removeMember(w http.ResponseWriter, r *http.Request) { return } - // remove from db - if err = db.RemoveSpindleMember( - tx, - orm.FilterEq("did", user.Did), - orm.FilterEq("instance", instance), - orm.FilterEq("subject", memberId.DID), - ); err != nil { - l.Error("failed to remove spindle member", "err", err) - fail() - return - } - - // remove from enforcer - if err = s.Enforcer.RemoveSpindleMember(instance, memberId.DID.String()); err != nil { - l.Error("failed to update ACLs", "err", err) - fail() - return - } - client, err := s.OAuth.AuthorizedClient(r) if err != nil { l.Error("failed to authorize client", "err", err) @@ -657,31 +585,16 @@ func (s *Spindles) removeMember(w http.ResponseWriter, r *http.Request) { return } - // remove from pds _, err = comatproto.RepoDeleteRecord(r.Context(), client, &comatproto.RepoDeleteRecord_Input{ Collection: tangled.SpindleMemberNSID, Repo: user.Did, Rkey: members[0].Rkey, }) if err != nil { - // non-fatal l.Error("failed to delete record", "err", err) - } - - // commit everything - if err = tx.Commit(); err != nil { - l.Error("failed to commit txn", "err", err) - fail() - return - } - - // commit everything - if err = s.Enforcer.E.SavePolicy(); err != nil { - l.Error("failed to save ACLs", "err", err) fail() return } - // ok s.Pages.HxRefresh(w) } diff --git a/knotserver/ingester.go b/knotserver/ingester.go index e77c1ca9..c0af3262 100644 --- a/knotserver/ingester.go +++ b/knotserver/ingester.go @@ -2,7 +2,9 @@ package knotserver import ( "context" + "database/sql" "encoding/json" + "errors" "fmt" "io" "net/http" @@ -49,40 +51,183 @@ func (h *Knot) processPublicKey(ctx context.Context, event *jmodels.Event) error } func (h *Knot) processKnotMember(ctx context.Context, event *jmodels.Event) error { - l := log.FromContext(ctx) - raw := json.RawMessage(event.Commit.Record) did := event.Did + rkey := event.Commit.RKey + l := log.FromContext(ctx).With("handler", "processKnotMember", "did", did, "rkey", rkey) + + switch event.Commit.Operation { + case jmodels.CommitOperationCreate, jmodels.CommitOperationUpdate: + raw := json.RawMessage(event.Commit.Record) + var record tangled.KnotMember + if err := json.Unmarshal(raw, &record); err != nil { + return fmt.Errorf("failed to unmarshal record: %w", err) + } - var record tangled.KnotMember - if err := json.Unmarshal(raw, &record); err != nil { - return fmt.Errorf("failed to unmarshal record: %w", err) - } + if record.Domain != h.c.Server.Hostname { + return fmt.Errorf("domain mismatch: %s != %s", record.Domain, h.c.Server.Hostname) + } - if record.Domain != h.c.Server.Hostname { - l.Error("domain mismatch", "domain", record.Domain, "expected", h.c.Server.Hostname) - return fmt.Errorf("domain mismatch: %s != %s", record.Domain, h.c.Server.Hostname) - } + subject, err := syntax.ParseDID(record.Subject) + if err != nil { + return fmt.Errorf("invalid subject DID %q: %w", record.Subject, err) + } - ok, err := h.e.E.Enforce(did, rbac.ThisServer, rbac.ThisServer, "server:invite") - if err != nil || !ok { - l.Error("failed to add member", "did", did) - return fmt.Errorf("failed to enforce permissions: %w", err) - } + ok, err := h.e.E.Enforce(did, rbac.ThisServer, rbac.ThisServer, "server:invite") + if err != nil { + return fmt.Errorf("failed to enforce permissions: %w", err) + } + if !ok { + return fmt.Errorf("permission denied for %s", did) + } - if err := h.e.AddKnotMember(rbac.ThisServer, record.Subject); err != nil { - l.Error("failed to add member", "error", err) - return fmt.Errorf("failed to add member: %w", err) - } - l.Info("added member from firehose", "member", record.Subject) + sqlTx, err := h.db.BeginTx(ctx, nil) + if err != nil { + return fmt.Errorf("failed to start txn: %w", err) + } + committed := false + defer func() { + if !committed { + sqlTx.Rollback() + } + }() + + existing, err := db.GetKnotMember(sqlTx, did, rkey) + if err != nil && !errors.Is(err, sql.ErrNoRows) { + return fmt.Errorf("failed to look up existing member: %w", err) + } - if err := h.db.AddDid(record.Subject); err != nil { - l.Error("failed to add did", "error", err) - return fmt.Errorf("failed to add did: %w", err) - } - h.jc.AddDid(record.Subject) + var staleSubject string + if existing != nil && existing.Subject != subject { + staleSubject = existing.Subject.String() + if err := db.RemoveKnotMember(sqlTx, did, rkey); err != nil { + return fmt.Errorf("failed to remove stale member row: %w", err) + } + } + + if err := db.AddKnotMember(sqlTx, db.KnotMember{ + Did: syntax.DID(did), + Rkey: rkey, + Subject: subject, + }); err != nil { + return fmt.Errorf("failed to persist member row: %w", err) + } + + if err := db.AddDid(sqlTx, subject.String()); err != nil { + return fmt.Errorf("failed to add did: %w", err) + } + + dropStaleAcl := false + var staleDidDropped bool + if staleSubject != "" { + remaining, err := db.CountKnotMembersBySubject(sqlTx, staleSubject) + if err != nil { + return fmt.Errorf("failed to count stale subject rows: %w", err) + } + if remaining == 0 { + dropStaleAcl = true + stillNeeded, err := h.e.WouldHaveAnyPolicyExcludingKnotMember(staleSubject, rbac.ThisServer) + if err != nil { + return fmt.Errorf("failed to check residual policies for stale subject: %w", err) + } + if !stillNeeded { + if err := db.RemoveDid(sqlTx, staleSubject); err != nil { + return fmt.Errorf("failed to remove stale did: %w", err) + } + staleDidDropped = true + } + } + l.Info("replaced stale knot member", "old_subject", staleSubject, "new_subject", subject, "stale_did_dropped", staleDidDropped) + } + + if err := sqlTx.Commit(); err != nil { + return fmt.Errorf("failed to commit txn: %w", err) + } + committed = true + + if dropStaleAcl { + if _, err := h.e.TryRemoveKnotMember(rbac.ThisServer, staleSubject); err != nil { + l.Error("post-commit: failed to remove stale ACL", "subject", staleSubject, "err", err) + } + } + if _, err := h.e.TryAddKnotMember(rbac.ThisServer, subject.String()); err != nil { + l.Error("post-commit: failed to add member ACL", "subject", subject, "err", err) + } + + if staleDidDropped { + h.jc.RemoveDid(staleSubject) + } + h.jc.AddDid(subject.String()) + l.Info("added member from firehose", "member", subject) + + if err := h.fetchAndAddKeys(ctx, subject.String()); err != nil { + return fmt.Errorf("failed to fetch and add keys: %w", err) + } + return nil + + case jmodels.CommitOperationDelete: + sqlTx, err := h.db.BeginTx(ctx, nil) + if err != nil { + return fmt.Errorf("failed to start txn: %w", err) + } + committed := false + defer func() { + if !committed { + sqlTx.Rollback() + } + }() + + member, err := db.GetKnotMember(sqlTx, did, rkey) + if errors.Is(err, sql.ErrNoRows) { + l.Info("knot member already removed") + return nil + } + if err != nil { + return fmt.Errorf("failed to look up knot member: %w", err) + } - if err := h.fetchAndAddKeys(ctx, record.Subject); err != nil { - return fmt.Errorf("failed to fetch and add keys: %w", err) + staleSubject := member.Subject.String() + + if err := db.RemoveKnotMember(sqlTx, did, rkey); err != nil { + return fmt.Errorf("failed to remove member row: %w", err) + } + + remaining, err := db.CountKnotMembersBySubject(sqlTx, staleSubject) + if err != nil { + return fmt.Errorf("failed to count remaining member rows: %w", err) + } + + dropAcl := false + var staleDidDropped bool + if remaining == 0 { + dropAcl = true + stillNeeded, err := h.e.WouldHaveAnyPolicyExcludingKnotMember(staleSubject, rbac.ThisServer) + if err != nil { + return fmt.Errorf("failed to check residual policies: %w", err) + } + if !stillNeeded { + if err := db.RemoveDid(sqlTx, staleSubject); err != nil { + return fmt.Errorf("failed to remove did: %w", err) + } + staleDidDropped = true + } + } + + if err := sqlTx.Commit(); err != nil { + return fmt.Errorf("failed to commit txn: %w", err) + } + committed = true + + if dropAcl { + if _, err := h.e.TryRemoveKnotMember(rbac.ThisServer, staleSubject); err != nil { + l.Error("post-commit: failed to remove ACL", "subject", staleSubject, "err", err) + } + } + + if staleDidDropped { + h.jc.RemoveDid(staleSubject) + } + l.Info("removed knot member from firehose", "member", member.Subject, "remaining_rows", remaining, "stale_did_dropped", staleDidDropped) + return nil } return nil @@ -437,7 +582,7 @@ func (h *Knot) processCollaborator(ctx context.Context, event *jmodels.Event) er return fmt.Errorf("insufficient permissions: %s, %s, %s", did, "IsCollaboratorInviteAllowed", rbacResource) } - if err := h.db.AddDid(subjectId.DID.String()); err != nil { + if err := db.AddDid(h.db, subjectId.DID.String()); err != nil { return err } h.jc.AddDid(subjectId.DID.String()) @@ -575,7 +720,7 @@ func (h *Knot) processMessages(ctx context.Context, event *jmodels.Event) error if err != nil { args := []any{"kind", event.Kind, "err", err} if event.Kind == jmodels.EventKindCommit { - args = append(args, "nsid", event.Commit.Collection) + args = append(args, "nsid", event.Commit.Collection, "did", event.Did, "rkey", event.Commit.RKey) } h.l.Warn("failed to process event, skipping", args...) } diff --git a/rbac/rbac.go b/rbac/rbac.go index 6887a648..9a64b18f 100644 --- a/rbac/rbac.go +++ b/rbac/rbac.go @@ -43,7 +43,7 @@ func NewEnforcer(path string) (*Enforcer, error) { return nil, err } - db, err := sql.Open("sqlite3", path+"?_foreign_keys=1") + db, err := sql.Open("sqlite3", path+"?_foreign_keys=1&_journal_mode=WAL&_busy_timeout=5000") if err != nil { return nil, err } diff --git a/rbac/util.go b/rbac/util.go index 41ad2495..bb2e06c4 100644 --- a/rbac/util.go +++ b/rbac/util.go @@ -72,6 +72,37 @@ func (e *Enforcer) HasAnyPolicyForUser(user string) (bool, error) { return len(gPolicies) > 0, nil } +func (e *Enforcer) wouldHaveAnyPolicyExcludingGrouping(user, role, domain string) (bool, error) { + pPolicies, err := e.E.GetFilteredNamedPolicy("p", 0, user) + if err != nil { + return false, err + } + if len(pPolicies) > 0 { + return true, nil + } + gPolicies, err := e.E.GetFilteredNamedGroupingPolicy("g", 0, user) + if err != nil { + return false, err + } + for _, gp := range gPolicies { + if len(gp) < 3 { + return true, nil + } + if gp[1] != role || gp[2] != domain { + return true, nil + } + } + return false, nil +} + +func (e *Enforcer) WouldHaveAnyPolicyExcludingKnotMember(user, domain string) (bool, error) { + return e.wouldHaveAnyPolicyExcludingGrouping(user, "server:member", domain) +} + +func (e *Enforcer) WouldHaveAnyPolicyExcludingSpindleMember(user, domain string) (bool, error) { + return e.wouldHaveAnyPolicyExcludingGrouping(user, "server:member", intoSpindle(domain)) +} + func checkRepoFormat(repo string) error { // sanity check, repo must be of the form ownerDid/repo if parts := strings.SplitN(repo, "/", 2); !strings.HasPrefix(parts[0], "did:") { diff --git a/spindle/ingester.go b/spindle/ingester.go index 6544a9b2..babae090 100644 --- a/spindle/ingester.go +++ b/spindle/ingester.go @@ -2,9 +2,10 @@ package spindle import ( "context" + "database/sql" "encoding/json" + "errors" "fmt" - "time" "tangled.org/core/api/tangled" "tangled.org/core/spindle/db" @@ -33,7 +34,7 @@ func (s *Spindle) ingest() Ingester { } if err != nil { - s.l.Warn("failed to process message, skipping", "nsid", e.Commit.Collection, "err", err) + s.l.Warn("failed to process message, skipping", "nsid", e.Commit.Collection, "did", e.Did, "rkey", e.Commit.RKey, "err", err) } lastTimeUs := e.TimeUS + 1 @@ -76,86 +77,183 @@ func jetstreamToTapEvent(e *models.Event) (tapc.Event, bool) { }, true } -func (s *Spindle) ingestMember(_ context.Context, e *models.Event) error { - var err error +func (s *Spindle) ingestMember(ctx context.Context, e *models.Event) error { did := e.Did rkey := e.Commit.RKey - - l := s.l.With("component", "ingester", "record", tangled.SpindleMemberNSID) + l := s.l.With("component", "ingester", "record", tangled.SpindleMemberNSID, "did", did, "rkey", rkey) switch e.Commit.Operation { case models.CommitOperationCreate, models.CommitOperationUpdate: raw := e.Commit.Record record := tangled.SpindleMember{} - err = json.Unmarshal(raw, &record) - if err != nil { - l.Error("invalid record", "error", err) - return err + if err := json.Unmarshal(raw, &record); err != nil { + return fmt.Errorf("invalid record: %w", err) } domain := s.cfg.Server.Hostname recordInstance := record.Instance if recordInstance != domain { - l.Error("domain mismatch", "domain", recordInstance, "expected", domain) return fmt.Errorf("domain mismatch: %s != %s", record.Instance, domain) } + subject, err := syntax.ParseDID(record.Subject) + if err != nil { + return fmt.Errorf("invalid subject DID %q: %w", record.Subject, err) + } + ok, err := s.e.IsSpindleInviteAllowed(did, rbacDomain) - if err != nil || !ok { - l.Error("failed to add member", "did", did, "error", err) + if err != nil { return fmt.Errorf("failed to enforce permissions: %w", err) } + if !ok { + return fmt.Errorf("permission denied for %s", did) + } + + sqlTx, err := s.db.BeginTx(ctx, nil) + if err != nil { + return fmt.Errorf("failed to start txn: %w", err) + } + committed := false + defer func() { + if !committed { + sqlTx.Rollback() + } + }() - if err := db.AddSpindleMember(s.db, db.SpindleMember{ + existing, err := db.GetSpindleMember(sqlTx, did, rkey) + if err != nil && !errors.Is(err, sql.ErrNoRows) { + return fmt.Errorf("failed to look up existing member: %w", err) + } + + var staleSubject string + if existing != nil && existing.Subject != subject { + staleSubject = existing.Subject.String() + if err := db.RemoveSpindleMember(sqlTx, did, rkey); err != nil { + return fmt.Errorf("failed to remove stale member row: %w", err) + } + } + + if err := db.AddSpindleMember(sqlTx, db.SpindleMember{ Did: syntax.DID(did), Rkey: rkey, Instance: recordInstance, - Subject: syntax.DID(record.Subject), - Created: time.Now(), + Subject: subject, }); err != nil { - l.Error("failed to add member", "error", err) return fmt.Errorf("failed to add member: %w", err) } - if err := s.e.AddSpindleMember(rbacDomain, record.Subject); err != nil { - l.Error("failed to add member", "error", err) - return fmt.Errorf("failed to add member: %w", err) + if err := db.AddDid(sqlTx, subject.String()); err != nil { + return fmt.Errorf("failed to add did: %w", err) } - l.Info("added member from firehose", "member", record.Subject) - if err := s.db.AddDid(record.Subject); err != nil { - l.Error("failed to add did", "error", err) - return fmt.Errorf("failed to add did: %w", err) + dropStaleAcl := false + var staleDidDropped bool + if staleSubject != "" { + remaining, err := db.CountSpindleMembersBySubject(sqlTx, staleSubject) + if err != nil { + return fmt.Errorf("failed to count stale subject rows: %w", err) + } + if remaining == 0 { + dropStaleAcl = true + stillNeeded, err := s.e.WouldHaveAnyPolicyExcludingSpindleMember(staleSubject, rbacDomain) + if err != nil { + return fmt.Errorf("failed to check residual policies for stale subject: %w", err) + } + if !stillNeeded { + if err := db.RemoveDid(sqlTx, staleSubject); err != nil { + return fmt.Errorf("failed to remove stale did: %w", err) + } + staleDidDropped = true + } + } + l.Info("replaced stale spindle member", "old_subject", staleSubject, "new_subject", subject, "stale_did_dropped", staleDidDropped) + } + + if err := sqlTx.Commit(); err != nil { + return fmt.Errorf("failed to commit txn: %w", err) } - s.jc.AddDid(record.Subject) + committed = true + if dropStaleAcl { + if _, err := s.e.TryRemoveSpindleMember(rbacDomain, staleSubject); err != nil { + l.Error("post-commit: failed to remove stale ACL", "subject", staleSubject, "err", err) + } + } + if _, err := s.e.TryAddSpindleMember(rbacDomain, subject.String()); err != nil { + l.Error("post-commit: failed to add member ACL", "subject", subject, "err", err) + } + + if staleDidDropped { + s.jc.RemoveDid(staleSubject) + } + s.jc.AddDid(subject.String()) + l.Info("added member from firehose", "member", subject) return nil case models.CommitOperationDelete: - record, err := db.GetSpindleMember(s.db, did, rkey) + sqlTx, err := s.db.BeginTx(ctx, nil) + if err != nil { + return fmt.Errorf("failed to start txn: %w", err) + } + committed := false + defer func() { + if !committed { + sqlTx.Rollback() + } + }() + + record, err := db.GetSpindleMember(sqlTx, did, rkey) + if errors.Is(err, sql.ErrNoRows) { + l.Info("spindle member already removed") + return nil + } if err != nil { - l.Error("failed to find member", "error", err) return fmt.Errorf("failed to find member: %w", err) } - if err := db.RemoveSpindleMember(s.db, did, rkey); err != nil { - l.Error("failed to remove member", "error", err) + staleSubject := record.Subject.String() + + if err := db.RemoveSpindleMember(sqlTx, did, rkey); err != nil { return fmt.Errorf("failed to remove member: %w", err) } - if err := s.e.RemoveSpindleMember(rbacDomain, record.Subject.String()); err != nil { - l.Error("failed to add member", "error", err) - return fmt.Errorf("failed to add member: %w", err) + remaining, err := db.CountSpindleMembersBySubject(sqlTx, staleSubject) + if err != nil { + return fmt.Errorf("failed to count remaining member rows: %w", err) } - l.Info("added member from firehose", "member", record.Subject) - if err := s.db.RemoveDid(record.Subject.String()); err != nil { - l.Error("failed to add did", "error", err) - return fmt.Errorf("failed to add did: %w", err) + dropAcl := false + var staleDidDropped bool + if remaining == 0 { + dropAcl = true + stillNeeded, err := s.e.WouldHaveAnyPolicyExcludingSpindleMember(staleSubject, rbacDomain) + if err != nil { + return fmt.Errorf("failed to check residual policies: %w", err) + } + if !stillNeeded { + if err := db.RemoveDid(sqlTx, staleSubject); err != nil { + return fmt.Errorf("failed to remove did: %w", err) + } + staleDidDropped = true + } } - s.jc.RemoveDid(record.Subject.String()) + if err := sqlTx.Commit(); err != nil { + return fmt.Errorf("failed to commit txn: %w", err) + } + committed = true + + if dropAcl { + if _, err := s.e.TryRemoveSpindleMember(rbacDomain, staleSubject); err != nil { + l.Error("post-commit: failed to remove member ACL", "subject", staleSubject, "err", err) + } + } + + if staleDidDropped { + s.jc.RemoveDid(staleSubject) + } + l.Info("removed member from firehose", "member", record.Subject, "remaining_rows", remaining, "stale_did_dropped", staleDidDropped) } return nil }