Skip to content

Commit 0b6a723

Browse files
committed
refactor: cut bot identity comment noise
1 parent 34cdc64 commit 0b6a723

8 files changed

Lines changed: 7 additions & 32 deletions

File tree

pkg/db/fixtures/labels.yml

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,8 +64,6 @@
6464
created_by_id: 21
6565
updated: 2018-12-02 15:13:12
6666
created: 2018-12-01 15:13:12
67-
# Created by bot 25, the sibling of bot 23 under user 21, with no task
68-
# attachment: only the identity branch can grant access to it.
6967
- id: 12
7068
title: 'Label #12 - created by bot 25, sibling of bot 23'
7169
created_by_id: 25

pkg/db/fixtures/users.yml

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -226,8 +226,6 @@
226226
bot_owner_id: 22
227227
updated: 2018-12-02 15:13:12
228228
created: 2018-12-01 15:13:12
229-
# Second bot owned by user 21 — sibling of bot 23, used to assert bots of the
230-
# same owner share an identity.
231229
- id: 25
232230
username: 'bot-owner-a-scheduler'
233231
name: 'Owner A Scheduler'

pkg/models/label_task_test.go

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -289,8 +289,6 @@ func TestLabelTask_Create(t *testing.T) {
289289
wantForbidden: true,
290290
},
291291
{
292-
// Label 12 was created by bot 25 and never attached: bot 23 reaches
293-
// it only through the owner both bots share.
294292
name: "bot can attach a label created by a sibling bot",
295293
fields: fields{
296294
TaskID: 52,

pkg/models/label_test.go

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -466,10 +466,7 @@ func TestLabel_ReadOne(t *testing.T) {
466466
auth: &user.User{ID: 24, BotOwnerID: 22},
467467
},
468468
{
469-
// Label 12 was created by bot 25, the sibling of bot 23 under user
470-
// 21, and is unattached. BotOwnerID is left unset here on purpose:
471-
// JWT-built auth values carry none, so the identity must come from
472-
// the database.
469+
// JWT auth carries no BotOwnerID, so the identity must resolve from the database.
473470
name: "bot can read label created by a sibling bot",
474471
fields: fields{
475472
ID: 12,
@@ -723,8 +720,6 @@ func TestLabel_Update(t *testing.T) {
723720
auth: &user.User{ID: 21},
724721
},
725722
{
726-
// Sharing an identity grants read access, not write: updates stay
727-
// with the creator and the human who owns it.
728723
name: "bot cannot update label created by a sibling bot",
729724
fields: fields{
730725
ID: 12,

pkg/user/bot_identity.go

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -20,26 +20,18 @@ import (
2020
"xorm.io/builder"
2121
)
2222

23-
// The two helpers below are not opposites: SameBotIdentityCond is symmetric
24-
// across an owner's fleet, IsBotOwnedBy only ever points from owner to bot.
25-
// Symmetry is opt-in per resource - so far only labels - and gates reads
26-
// alone. Everything else, including reading bot users and their tokens, stays
27-
// on the directional check.
28-
2923
// IsBotOwnedBy reports whether u is a bot owned by owner.
3024
func (u *User) IsBotOwnedBy(owner *User) bool {
3125
return u.IsBot() && u.BotOwnerID == owner.ID
3226
}
3327

34-
// SameBotIdentityCond matches a user id column against every user sharing u's
35-
// identity root - a bot's owner, or a human themselves.
28+
// SameBotIdentityCond matches a user ID column against users sharing u's root:
29+
// a bot's owner or a human themselves. It is symmetric across an owner's fleet.
3630
//
3731
// The root is resolved in SQL so the result never depends on how populated the
3832
// passed struct happens to be; callers routinely hold a User carrying only an ID.
3933
//
40-
// Chain-safe by construction rather than by invariant: the root is exactly one
41-
// hop up, so even a bot owning a bot would leave the grandchild resolving to its
42-
// parent bot, never reaching the human.
34+
// Resolving only one owner hop keeps accidental bot chains out of a human's identity set.
4335
//
4436
// column is interpolated as a raw SQL identifier and must be a trusted literal.
4537
func SameBotIdentityCond(u *User, column string) builder.Cond {

pkg/user/bot_identity_test.go

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,6 @@ func TestUser_IsBotOwnedBy(t *testing.T) {
5757
want: false,
5858
},
5959
{
60-
// Reversed direction: the bot must not inherit its owner.
6160
name: "caller is the bot, subject is the owner",
6261
u: &User{ID: 21},
6362
a: &User{ID: 23, BotOwnerID: 21},
@@ -70,8 +69,7 @@ func TestUser_IsBotOwnedBy(t *testing.T) {
7069
want: false,
7170
},
7271
{
73-
// Both ids are 0, so only the IsBot() half keeps a human from being
74-
// reported as owned by an unresolved caller.
72+
// IsBot prevents a zero-ID owner from matching a human.
7573
name: "human subject, zero-id owner",
7674
u: &User{ID: 5},
7775
a: &User{},
@@ -89,8 +87,7 @@ func TestUser_IsBotOwnedBy(t *testing.T) {
8987
func TestSameBotIdentityCond(t *testing.T) {
9088
tests := []struct {
9189
name string
92-
// Only the ID is set on purpose: the identity must be resolved from
93-
// the database, not from the in-memory struct.
90+
// JWT auth carries only an ID, so the identity must resolve from the database.
9491
auth *User
9592
want []int64
9693
}{

pkg/webtests/huma_bot_user_test.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,6 @@ func TestHumaBotUser(t *testing.T) {
7272
rec, err := h.testReadAllWithUser(nil, nil)
7373
require.NoError(t, err)
7474
ids := botIDsFromReadAll(t, rec.Body.Bytes())
75-
// user 21 owns exactly bots 23 and 25; user 22's bot 24 must never leak.
7675
assert.ElementsMatch(t, []int64{23, 25}, ids,
7776
"ReadAll must return exactly {23,25}; body: %s", rec.Body.String())
7877
assert.NotContains(t, ids, int64(24), "bot #24 (other owner) must be hidden")

pkg/webtests/huma_label_test.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -302,9 +302,7 @@ func TestHumaLabel_BotOwner(t *testing.T) {
302302
})
303303
}
304304

305-
// Fixture labels #11 (created by the owner) and #12 (created by sibling bot 25)
306-
// are unattached, so attachment history cannot grant access here - only the
307-
// shared identity can (#3592).
305+
// Unattached labels #11 and #12 isolate identity-based access (#3592).
308306
func TestHumaLabel_BotUsesOwnerLabel(t *testing.T) {
309307
bot := webHandlerTestV2{
310308
user: &testbot23,

0 commit comments

Comments
 (0)