Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit72d9876

Browse files
authored
fix(coderd/workspaceapps): prevent race in workspace app audit session updates (#17020)
Fixescoder/internal#520
1 parent6862409 commit72d9876

File tree

13 files changed

+68
-32
lines changed

13 files changed

+68
-32
lines changed

‎coderd/database/dbauthz/dbauthz.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4625,9 +4625,9 @@ func (q *querier) UpsertWorkspaceAgentPortShare(ctx context.Context, arg databas
46254625
returnq.db.UpsertWorkspaceAgentPortShare(ctx,arg)
46264626
}
46274627

4628-
func (q*querier)UpsertWorkspaceAppAuditSession(ctx context.Context,arg database.UpsertWorkspaceAppAuditSessionParams) (time.Time,error) {
4628+
func (q*querier)UpsertWorkspaceAppAuditSession(ctx context.Context,arg database.UpsertWorkspaceAppAuditSessionParams) (bool,error) {
46294629
iferr:=q.authorizeContext(ctx,policy.ActionUpdate,rbac.ResourceSystem);err!=nil {
4630-
returntime.Time{},err
4630+
returnfalse,err
46314631
}
46324632
returnq.db.UpsertWorkspaceAppAuditSession(ctx,arg)
46334633
}

‎coderd/database/dbmem/dbmem.go

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12298,10 +12298,10 @@ func (q *FakeQuerier) UpsertWorkspaceAgentPortShare(_ context.Context, arg datab
1229812298
returnpsl,nil
1229912299
}
1230012300

12301-
func (q*FakeQuerier)UpsertWorkspaceAppAuditSession(_ context.Context,arg database.UpsertWorkspaceAppAuditSessionParams) (time.Time,error) {
12301+
func (q*FakeQuerier)UpsertWorkspaceAppAuditSession(_ context.Context,arg database.UpsertWorkspaceAppAuditSessionParams) (bool,error) {
1230212302
err:=validateDatabaseType(arg)
1230312303
iferr!=nil {
12304-
returntime.Time{},err
12304+
returnfalse,err
1230512305
}
1230612306

1230712307
q.mutex.Lock()
@@ -12335,10 +12335,11 @@ func (q *FakeQuerier) UpsertWorkspaceAppAuditSession(_ context.Context, arg data
1233512335

1233612336
q.workspaceAppAuditSessions[i].UpdatedAt=arg.UpdatedAt
1233712337
if!fresh {
12338+
q.workspaceAppAuditSessions[i].ID=arg.ID
1233812339
q.workspaceAppAuditSessions[i].StartedAt=arg.StartedAt
12339-
returnarg.StartedAt,nil
12340+
returntrue,nil
1234012341
}
12341-
returns.StartedAt,nil
12342+
returnfalse,nil
1234212343
}
1234312344

1234412345
q.workspaceAppAuditSessions=append(q.workspaceAppAuditSessions, database.WorkspaceAppAuditSession{
@@ -12352,7 +12353,7 @@ func (q *FakeQuerier) UpsertWorkspaceAppAuditSession(_ context.Context, arg data
1235212353
StartedAt:arg.StartedAt,
1235312354
UpdatedAt:arg.UpdatedAt,
1235412355
})
12355-
returnarg.StartedAt,nil
12356+
returntrue,nil
1235612357
}
1235712358

1235812359
func (q*FakeQuerier)GetAuthorizedTemplates(ctx context.Context,arg database.GetTemplatesWithFilterParams,prepared rbac.PreparedAuthorized) ([]database.Template,error) {

‎coderd/database/dbmetrics/querymetrics.go

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more aboutcustomizing how changed files appear on GitHub.

‎coderd/database/dbmock/dbmock.go

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more aboutcustomizing how changed files appear on GitHub.

‎coderd/database/dump.sql

Lines changed: 5 additions & 1 deletion
Some generated files are not rendered by default. Learn more aboutcustomizing how changed files appear on GitHub.
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
ALTERTABLE workspace_app_audit_sessions
2+
DROP COLUMN id;
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
-- Add column with default to fix existing rows.
2+
ALTERTABLE workspace_app_audit_sessions
3+
ADD COLUMN id UUIDPRIMARY KEY DEFAULT gen_random_uuid();
4+
ALTERTABLE workspace_app_audit_sessions
5+
ALTER COLUMN id DROP DEFAULT;

‎coderd/database/models.go

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more aboutcustomizing how changed files appear on GitHub.

‎coderd/database/querier.go

Lines changed: 4 additions & 3 deletions
Some generated files are not rendered by default. Learn more aboutcustomizing how changed files appear on GitHub.

‎coderd/database/queries.sql.go

Lines changed: 20 additions & 9 deletions
Some generated files are not rendered by default. Learn more aboutcustomizing how changed files appear on GitHub.

‎coderd/database/queries/workspaceappaudit.sql

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
11
-- name: UpsertWorkspaceAppAuditSession :one
22
--
3-
-- Insert a new workspace app audit session or update an existing one, if
4-
-- started_at is updated, it means the session has been restarted.
3+
-- The returned boolean, new_or_stale, can be used to deduce if a new session
4+
-- was started. This means that a new row was inserted (no previous session) or
5+
-- the updated_at is older than stale interval.
56
INSERT INTO
67
workspace_app_audit_sessions (
8+
id,
79
agent_id,
810
app_id,
911
user_id,
@@ -24,18 +26,25 @@ VALUES
2426
$6,
2527
$7,
2628
$8,
27-
$9
29+
$9,
30+
$10
2831
)
2932
ON CONFLICT
3033
(agent_id, app_id, user_id, ip, user_agent, slug_or_port, status_code)
3134
DO
3235
UPDATE
3336
SET
37+
-- ID is used to know if session was reset on upsert.
38+
id= CASE
39+
WHENworkspace_app_audit_sessions.updated_at> NOW()- (@stale_interval_ms::bigint||' ms')::interval
40+
THENworkspace_app_audit_sessions.id
41+
ELSEEXCLUDED.id
42+
END,
3443
started_at= CASE
3544
WHENworkspace_app_audit_sessions.updated_at> NOW()- (@stale_interval_ms::bigint||' ms')::interval
3645
THENworkspace_app_audit_sessions.started_at
3746
ELSEEXCLUDED.started_at
3847
END,
3948
updated_at=EXCLUDED.updated_at
4049
RETURNING
41-
started_at;
50+
id= $1AS new_or_stale;

‎coderd/database/unique_constraint.go

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more aboutcustomizing how changed files appear on GitHub.

‎coderd/workspaceapps/db.go

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -447,16 +447,17 @@ func (p *DBTokenProvider) auditInitRequest(ctx context.Context, w http.ResponseW
447447
slog.F("status_code",statusCode),
448448
)
449449

450-
varstartedAt time.Time
450+
varnewOrStalebool
451451
err:=p.Database.InTx(func(tx database.Store) (errerror) {
452452
// nolint:gocritic // System context is needed to write audit sessions.
453453
dangerousSystemCtx:=dbauthz.AsSystemRestricted(ctx)
454454

455-
startedAt,err=tx.UpsertWorkspaceAppAuditSession(dangerousSystemCtx, database.UpsertWorkspaceAppAuditSessionParams{
455+
newOrStale,err=tx.UpsertWorkspaceAppAuditSession(dangerousSystemCtx, database.UpsertWorkspaceAppAuditSessionParams{
456456
// Config.
457457
StaleIntervalMS:p.WorkspaceAppAuditSessionTimeout.Milliseconds(),
458458

459459
// Data.
460+
ID:uuid.New(),
460461
AgentID:aReq.dbReq.Agent.ID,
461462
AppID:aReq.dbReq.App.ID,// Can be unset, in which case uuid.Nil is fine.
462463
UserID:userID,// Can be unset, in which case uuid.Nil is fine.
@@ -481,9 +482,9 @@ func (p *DBTokenProvider) auditInitRequest(ctx context.Context, w http.ResponseW
481482
return
482483
}
483484

484-
if!startedAt.Equal(aReq.time) {
485-
//If the unique session wasn'trenewed, we don't want to log a new
486-
//audit event for it.
485+
if!newOrStale {
486+
//We either didn'tinsert a new session, or the session
487+
//didn't timeout due to inactivity.
487488
return
488489
}
489490

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp