Skip to content

Commit 9b4f99b

Browse files
bbrksCopilot
andauthored
CBG-4767 2: Make AuditIDDocumentRead tolerant to invalid SyncData in _raw endpoint (#7653)
Co-authored-by: Copilot <[email protected]>
1 parent d4e2176 commit 9b4f99b

File tree

3 files changed

+23
-6
lines changed

3 files changed

+23
-6
lines changed

base/logger_audit.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,12 @@ func (f AuditFields) merge(ctx context.Context, overwrites AuditFields) AuditFie
115115
return f
116116
}
117117

118+
// AuditEventIsEnabled checks if the given audit event ID is enabled for the current context.
119+
// This may be used to avoid the cost of some data processing/gathering ahead of an audit event.
120+
func AuditEventIsEnabled(ctx context.Context, id AuditID) bool {
121+
return auditLogger.Load().shouldLog(id, ctx)
122+
}
123+
118124
// Audit creates and logs an audit event for the given ID and a set of additional data associated with the request.
119125
func Audit(ctx context.Context, id AuditID, additionalData AuditFields) {
120126
var fields AuditFields

rest/admin_api.go

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1471,17 +1471,20 @@ func (h *handler) handleGetRawDoc() error {
14711471
base.Audit(h.ctx(), base.AuditIDDocumentMetadataRead, base.AuditFields{
14721472
base.AuditFieldDocID: docID,
14731473
})
1474-
if includeDoc {
1475-
// try to extract a revision from the sync data
1474+
if includeDoc && base.AuditEventIsEnabled(h.ctx(), base.AuditIDDocumentRead) {
1475+
// best-effort attempt to extract a revision from the raw sync data
14761476
docCurrentRev := "unknown"
14771477
if syncData, ok := xattrValues[base.SyncXattrName]; ok {
14781478
var sdRev struct {
14791479
Rev channels.RevAndVersion `json:"rev"`
14801480
}
14811481
if err := json.Unmarshal(syncData, &sdRev); err != nil {
1482-
return base.HTTPErrorf(http.StatusInternalServerError, "couldn't unmarshal rev from SyncData for doc %q: %s", base.UD(docID), err)
1482+
// syncData is not normally generally redacted, but in this case we know it's malformed in some way and want the raw data in the log message.
1483+
// This data may contain channel names, etc. which _are_ classed as UserData, so tag the whole thing.
1484+
base.WarnfCtx(h.ctx(), "couldn't unmarshal doc %q SyncData xattr value %q: %s", base.UD(docID), base.UD(syncData), err)
1485+
} else {
1486+
docCurrentRev = sdRev.Rev.RevTreeID
14831487
}
1484-
docCurrentRev = sdRev.Rev.RevTreeID
14851488
}
14861489
base.Audit(h.ctx(), base.AuditIDDocumentRead, base.AuditFields{
14871490
base.AuditFieldDocID: docID,

rest/api_test.go

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1855,12 +1855,20 @@ func TestChanCacheActiveRevsStat(t *testing.T) {
18551855

18561856
}
18571857

1858-
func TestGetRawDocumentError(t *testing.T) {
1858+
func TestGetRawDocumentErrors(t *testing.T) {
18591859
rt := NewRestTester(t, nil)
18601860
defer rt.Close()
18611861

1862-
response := rt.SendAdminRequest("GET", "/{{.keyspace}}/_raw/doc", ``)
1862+
// not found
1863+
response := rt.SendAdminRequest(http.MethodGet, "/{{.keyspace}}/_raw/"+t.Name(), ``)
18631864
assert.Equal(t, http.StatusNotFound, response.Code)
1865+
1866+
// invalid sync data
1867+
_, err := rt.GetSingleDataStore().WriteWithXattrs(t.Context(), t.Name()+"invalsyncdata", 0, 0, []byte(`{"doc":"body"}`),
1868+
map[string][]byte{base.SyncXattrName: []byte(`"invalid sync data"`)}, nil, nil)
1869+
require.NoError(t, err)
1870+
response = rt.SendAdminRequest(http.MethodGet, "/{{.keyspace}}/_raw/"+t.Name()+"invalsyncdata", ``)
1871+
assert.Equal(t, http.StatusOK, response.Code)
18641872
}
18651873

18661874
func TestWebhookProperties(t *testing.T) {

0 commit comments

Comments
 (0)