From 2a5a6d65c4baf0e2177cff5a4baf155503e233cc Mon Sep 17 00:00:00 2001 From: David Dworken Date: Fri, 22 Sep 2023 13:13:46 -0700 Subject: [PATCH 01/12] Roll-forward "Add preliminary support for persisting pre-saved history entries remotely" This rolls-forward commit 66916c27cb36cc2ed941d93b573b196de3a5c60a. --- backend/server/internal/database/db.go | 11 ++++- backend/server/internal/server/server_test.go | 4 +- client/client_test.go | 27 ++++++++---- client/cmd/redact.go | 4 +- client/cmd/saveHistoryEntry.go | 41 +++++++++++-------- client/lib/lib.go | 5 ++- client/tui/tui.go | 4 +- shared/data.go | 22 ++++++---- 8 files changed, 76 insertions(+), 42 deletions(-) diff --git a/backend/server/internal/database/db.go b/backend/server/internal/database/db.go index 602a2ee..6c6e520 100644 --- a/backend/server/internal/database/db.go +++ b/backend/server/internal/database/db.go @@ -152,7 +152,14 @@ func (db *DB) DumpRequestDeleteForUserAndDevice(ctx context.Context, userID, dev func (db *DB) ApplyDeletionRequestsToBackend(ctx context.Context, request *shared.DeletionRequest) (int64, error) { tx := db.WithContext(ctx).Where("false") for _, message := range request.Messages.Ids { - tx = tx.Or(db.WithContext(ctx).Where("user_id = ? AND device_id = ? AND date = ?", request.UserId, message.DeviceId, message.Date)) + // Note that this won't do server-side deletion of pre-saved history entries. This is an inherent + // limitation of our current DB schema. This is sub-par, since it means that even after deletion, clients + // may still receive deleted history entries. But, a well-behaved client will immediately delete + // these (never writing them to disk) and mark them as received, so this won't happen again. + // + // TODO: This could be improved upon if we added a HistoryEntry.EntryId field, backfilled it, added + // it to EncHistoryEntry, and then used that as a key for deletion. + tx = tx.Or(db.WithContext(ctx).Where("user_id = ? AND device_id = ? AND date = ?", request.UserId, message.DeviceId, message.EndTime)) } result := tx.Delete(&shared.EncHistoryEntry{}) if tx.Error != nil { @@ -226,7 +233,7 @@ func (db *DB) Clean(ctx context.Context) error { if r.Error != nil { return r.Error } - r = db.WithContext(ctx).Exec("DELETE FROM deletion_requests WHERE read_count > 100") + r = db.WithContext(ctx).Exec("DELETE FROM deletion_requests WHERE read_count > 1000") if r.Error != nil { return r.Error } diff --git a/backend/server/internal/server/server_test.go b/backend/server/internal/server/server_test.go index b5ca045..0f706e6 100644 --- a/backend/server/internal/server/server_test.go +++ b/backend/server/internal/server/server_test.go @@ -411,7 +411,7 @@ func TestDeletionRequests(t *testing.T) { UserId: data.UserId("dkey"), SendTime: delReqTime, Messages: shared.MessageIdentifiers{Ids: []shared.MessageIdentifier{ - {DeviceId: devId1, Date: entry1.EndTime}, + {DeviceId: devId1, EndTime: entry1.EndTime}, }}, } reqBody, err = json.Marshal(delReq) @@ -507,7 +507,7 @@ func TestDeletionRequests(t *testing.T) { SendTime: delReqTime, ReadCount: 1, Messages: shared.MessageIdentifiers{Ids: []shared.MessageIdentifier{ - {DeviceId: devId1, Date: entry1.EndTime}, + {DeviceId: devId1, EndTime: entry1.EndTime}, }}, } if diff := deep.Equal(*deletionRequest, expected); diff != nil { diff --git a/client/client_test.go b/client/client_test.go index 513aeff..ac80335 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -2364,15 +2364,24 @@ func testPresaving(t *testing.T, tester shellTester) { out = tester.RunInteractiveShell(t, ` hishtory query sleep 13371337 -export -tquery`) testutils.CompareGoldens(t, out, "testPresaving-query") - // And the same for tquery - // out = captureTerminalOutputWithComplexCommands(t, tester, - // []TmuxCommand{ - // {Keys: "hishtory SPACE tquery ENTER", ExtraDelay: 2.0}, - // {Keys: "sleep SPACE 13371337 SPACE -export SPACE -tquery"}}) - // out = strings.TrimSpace(strings.Split(out, "hishtory tquery")[1]) - // testutils.CompareGoldens(t, out, "testPresaving-tquery") - // - // TODO: Debug why ^ is failing with flaky differences on Github Actions, see https://pastebin.com/BUa1btnh + // Create a new device, and confirm it shows up there too + restoreDevice1 := testutils.BackupAndRestoreWithId(t, "device1") + installHishtory(t, tester, userSecret) + tester.RunInteractiveShell(t, ` hishtory config-set displayed-columns CWD Runtime Command`) + out = tester.RunInteractiveShell(t, ` hishtory query sleep 13371337 -export -tquery`) + testutils.CompareGoldens(t, out, "testPresaving-query") + + // And then redact it from device2 + tester.RunInteractiveShell(t, ` HISHTORY_REDACT_FORCE=true hishtory redact sleep 13371337`) + + // And confirm it was redacted + out = tester.RunInteractiveShell(t, ` hishtory export sleep -export`) + require.Equal(t, "", out) + + // Then go back to device1 and confirm it was redacted there too + restoreDevice1() + out = tester.RunInteractiveShell(t, ` hishtory export sleep -export`) + require.Equal(t, "", out) } func testUninstall(t *testing.T, tester shellTester) { diff --git a/client/cmd/redact.go b/client/cmd/redact.go index aadb8e0..a79903e 100644 --- a/client/cmd/redact.go +++ b/client/cmd/redact.go @@ -86,7 +86,9 @@ func deleteOnRemoteInstances(ctx context.Context, historyEntries []*data.History deletionRequest.UserId = data.UserId(config.UserSecret) for _, entry := range historyEntries { - deletionRequest.Messages.Ids = append(deletionRequest.Messages.Ids, shared.MessageIdentifier{Date: entry.EndTime, DeviceId: entry.DeviceId}) + deletionRequest.Messages.Ids = append(deletionRequest.Messages.Ids, + shared.MessageIdentifier{StartTime: entry.StartTime, EndTime: entry.EndTime, DeviceId: entry.DeviceId}, + ) } return lib.SendDeletionRequest(deletionRequest) } diff --git a/client/cmd/saveHistoryEntry.go b/client/cmd/saveHistoryEntry.go index 2bd0ec2..9922c74 100644 --- a/client/cmd/saveHistoryEntry.go +++ b/client/cmd/saveHistoryEntry.go @@ -55,6 +55,7 @@ func maybeUploadSkippedHistoryEntries(ctx context.Context) error { // Upload the missing entries db := hctx.GetDb(ctx) + // TODO: There is a bug here because MissedUploadTimestamp is going to be a second or two after the history entry that needs to be uploaded query := fmt.Sprintf("after:%s", time.Unix(config.MissedUploadTimestamp, 0).Format("2006-01-02")) entries, err := lib.Search(ctx, db, query, 0) if err != nil { @@ -81,6 +82,21 @@ func maybeUploadSkippedHistoryEntries(ctx context.Context) error { return nil } +func handlePotentialUploadFailure(err error, config *hctx.ClientConfig) { + if err != nil { + if lib.IsOfflineError(err) { + hctx.GetLogger().Infof("Failed to remotely persist hishtory entry because we failed to connect to the remote server! This is likely because the device is offline, but also could be because the remote server is having reliability issues. Original error: %v", err) + if !config.HaveMissedUploads { + config.HaveMissedUploads = true + config.MissedUploadTimestamp = time.Now().Unix() + lib.CheckFatalError(hctx.SetConfig(*config)) + } + } else { + lib.CheckFatalError(err) + } + } +} + func presaveHistoryEntry(ctx context.Context) { config := hctx.GetConf(ctx) if !config.IsEnabled { @@ -119,12 +135,13 @@ func presaveHistoryEntry(ctx context.Context) { lib.CheckFatalError(err) db.Commit() - // Note that we aren't persisting these half-entries remotely, - // since they should be updated with the rest of the information very soon. If they - // are never updated (e.g. due to a long-running command that never finishes), then - // they will only be available on this device. That isn't perfect since it means - // history entries can get out of sync, but it is probably good enough. - // TODO: Consider improving this + // And persist it remotely + if !config.IsOffline { + jsonValue, err := lib.EncryptAndMarshal(config, []*data.HistoryEntry{entry}) + lib.CheckFatalError(err) + _, err = lib.ApiPost("/api/v1/submit?source_device_id="+config.DeviceId, "application/json", jsonValue) + handlePotentialUploadFailure(err, &config) + } } func saveHistoryEntry(ctx context.Context) { @@ -175,6 +192,7 @@ func saveHistoryEntry(ctx context.Context) { jsonValue, err := lib.EncryptAndMarshal(config, []*data.HistoryEntry{entry}) lib.CheckFatalError(err) w, err := lib.ApiPost("/api/v1/submit?source_device_id="+config.DeviceId, "application/json", jsonValue) + handlePotentialUploadFailure(err, &config) if err == nil { submitResponse := shared.SubmitResponse{} err := json.Unmarshal(w, &submitResponse) @@ -183,17 +201,6 @@ func saveHistoryEntry(ctx context.Context) { } shouldCheckForDeletionRequests = submitResponse.HaveDeletionRequests shouldCheckForDumpRequests = submitResponse.HaveDumpRequests - } else { - if lib.IsOfflineError(err) { - hctx.GetLogger().Infof("Failed to remotely persist hishtory entry because we failed to connect to the remote server! This is likely because the device is offline, but also could be because the remote server is having reliability issues. Original error: %v", err) - if !config.HaveMissedUploads { - config.HaveMissedUploads = true - config.MissedUploadTimestamp = time.Now().Unix() - lib.CheckFatalError(hctx.SetConfig(config)) - } - } else { - lib.CheckFatalError(err) - } } } diff --git a/client/lib/lib.go b/client/lib/lib.go index 9a4faa7..4caf710 100644 --- a/client/lib/lib.go +++ b/client/lib/lib.go @@ -600,7 +600,10 @@ func ProcessDeletionRequests(ctx context.Context) error { db := hctx.GetDb(ctx) for _, request := range deletionRequests { for _, entry := range request.Messages.Ids { - res := db.Where("device_id = ? AND end_time = ?", entry.DeviceId, entry.Date).Delete(&data.HistoryEntry{}) + // Note that entry.StartTime is not always present (for legacy reasons) and entry.EndTime is also + // not always present (for pre-saved entries). So we just check that one of them matches. + tx := db.Where("device_id = ? AND (start_time = ? OR end_time = ?)", entry.DeviceId, entry.StartTime, entry.EndTime) + res := tx.Delete(&data.HistoryEntry{}) if res.Error != nil { return fmt.Errorf("DB error: %w", res.Error) } diff --git a/client/tui/tui.go b/client/tui/tui.go index e0d73fa..ed65dcf 100644 --- a/client/tui/tui.go +++ b/client/tui/tui.go @@ -601,7 +601,9 @@ func deleteHistoryEntry(ctx context.Context, entry data.HistoryEntry) error { UserId: data.UserId(hctx.GetConf(ctx).UserSecret), SendTime: time.Now(), } - dr.Messages.Ids = append(dr.Messages.Ids, shared.MessageIdentifier{Date: entry.EndTime, DeviceId: entry.DeviceId}) + dr.Messages.Ids = append(dr.Messages.Ids, + shared.MessageIdentifier{StartTime: entry.StartTime, EndTime: entry.EndTime, DeviceId: entry.DeviceId}, + ) return lib.SendDeletionRequest(dr) } diff --git a/shared/data.go b/shared/data.go index 2a59282..0c6cc79 100644 --- a/shared/data.go +++ b/shared/data.go @@ -9,13 +9,14 @@ import ( // Represents an encrypted history entry type EncHistoryEntry struct { - EncryptedData []byte `json:"enc_data"` - Nonce []byte `json:"nonce"` - DeviceId string `json:"device_id"` - UserId string `json:"user_id"` - Date time.Time `json:"time"` - EncryptedId string `json:"id"` - ReadCount int `json:"read_count"` + EncryptedData []byte `json:"enc_data"` + Nonce []byte `json:"nonce"` + DeviceId string `json:"device_id"` + UserId string `json:"user_id"` + // Note that EncHistoryEntry.Date == HistoryEntry.EndTime + Date time.Time `json:"time"` + EncryptedId string `json:"id"` + ReadCount int `json:"read_count"` } /* @@ -87,8 +88,11 @@ type MessageIdentifiers struct { type MessageIdentifier struct { // The device that the entry was recorded on (NOT the device where it is stored/requesting deletion) DeviceId string `json:"device_id"` - // The timestamp when the message finished running - Date time.Time `json:"date"` + // The timestamp when the command finished running. Serialized as "date" for legacy compatibility. + EndTime time.Time `json:"date"` + // The timestamp when the command started running. + // Note this field was added as part of supporting pre-saving commands, so older clients do not set this field + StartTime time.Time `json:"start_time"` } func (m *MessageIdentifiers) Scan(value interface{}) error { From a5f11af15076a330b485f5d9c7416a0cd01c95ce Mon Sep 17 00:00:00 2001 From: David Dworken Date: Fri, 22 Sep 2023 13:16:24 -0700 Subject: [PATCH 02/12] Add initial code to support unique per-entry IDs This code has two caveats for old entries: 1. the ID is being backfiled with a random per-(device,entry) ID. So the ID won't match cross-device. 2. the server-side ID will still be a random ID that is unrelated to the entry ID --- client/cmd/install.go | 12 ++++++++++++ client/cmd/saveHistoryEntry.go | 4 ++++ client/data/data.go | 4 ++-- client/hctx/hctx.go | 1 + client/lib/lib.go | 1 + shared/testutils/testutils.go | 2 ++ 6 files changed, 22 insertions(+), 2 deletions(-) diff --git a/client/cmd/install.go b/client/cmd/install.go index 2d12b99..5eb5def 100644 --- a/client/cmd/install.go +++ b/client/cmd/install.go @@ -171,9 +171,21 @@ func install(secretKey string, offline bool) error { // No config, so set up a new installation return lib.Setup(secretKey, offline) } + err = handleDbUpgrades(hctx.MakeContext()) + if err != nil { + return err + } return nil } +// Handles people running `hishtory update` when the DB needs updating. +func handleDbUpgrades(ctx context.Context) error { + db := hctx.GetDb(ctx) + return db.Exec(` + UPDATE history_entries SET entry_id = lower(hex(randomblob(12))) WHERE entry_id IS NULL + `).Error +} + // Handles people running `hishtory update` from an old version of hishtory that // doesn't support the control-r integration, so that they'll get control-r enabled // but someone who has it explicitly disabled will keep it that way. diff --git a/client/cmd/saveHistoryEntry.go b/client/cmd/saveHistoryEntry.go index 9922c74..56c6fe3 100644 --- a/client/cmd/saveHistoryEntry.go +++ b/client/cmd/saveHistoryEntry.go @@ -18,6 +18,7 @@ import ( "github.com/ddworken/hishtory/client/hctx" "github.com/ddworken/hishtory/client/lib" "github.com/ddworken/hishtory/shared" + "github.com/google/uuid" "github.com/spf13/cobra" ) @@ -281,6 +282,9 @@ func buildPreArgsHistoryEntry(ctx context.Context) (*data.HistoryEntry, error) { config := hctx.GetConf(ctx) entry.DeviceId = config.DeviceId + // entry ID + entry.EntryId = uuid.Must(uuid.NewRandom()).String() + // custom columns cc, err := buildCustomColumns(ctx) if err != nil { diff --git a/client/data/data.go b/client/data/data.go index cea1035..6f1a812 100644 --- a/client/data/data.go +++ b/client/data/data.go @@ -15,7 +15,6 @@ import ( "time" "github.com/ddworken/hishtory/shared" - "github.com/google/uuid" ) const ( @@ -39,6 +38,7 @@ type HistoryEntry struct { StartTime time.Time `json:"start_time" gorm:"uniqueIndex:compositeindex"` EndTime time.Time `json:"end_time" gorm:"uniqueIndex:compositeindex,index:end_time_index"` DeviceId string `json:"device_id" gorm:"uniqueIndex:compositeindex"` + EntryId string `json:"entry_id" gorm:"uniqueIndex:compositeindex,uniqueIndex:entry_id_index"` CustomColumns CustomColumns `json:"custom_columns"` } @@ -136,7 +136,7 @@ func EncryptHistoryEntry(userSecret string, entry HistoryEntry) (shared.EncHisto Nonce: nonce, UserId: UserId(userSecret), Date: entry.EndTime, - EncryptedId: uuid.Must(uuid.NewRandom()).String(), + EncryptedId: entry.EntryId, ReadCount: 0, }, nil } diff --git a/client/hctx/hctx.go b/client/hctx/hctx.go index 846a697..ccb9c0b 100644 --- a/client/hctx/hctx.go +++ b/client/hctx/hctx.go @@ -103,6 +103,7 @@ func OpenLocalSqliteDb() (*gorm.DB, error) { db.AutoMigrate(&data.HistoryEntry{}) db.Exec("PRAGMA journal_mode = WAL") db.Exec("CREATE INDEX IF NOT EXISTS end_time_index ON history_entries(end_time)") + db.Exec("CREATE INDEX IF NOT EXISTS entry_id_index ON history_entries(entry_id)") return db, nil } diff --git a/client/lib/lib.go b/client/lib/lib.go index 4caf710..41e9523 100644 --- a/client/lib/lib.go +++ b/client/lib/lib.go @@ -303,6 +303,7 @@ func ImportHistory(ctx context.Context, shouldReadStdin, force bool) (int, error StartTime: time.Now().UTC(), EndTime: time.Now().UTC(), DeviceId: config.DeviceId, + EntryId: uuid.Must(uuid.NewRandom()).String(), } err = ReliableDbCreate(db, entry) if err != nil { diff --git a/shared/testutils/testutils.go b/shared/testutils/testutils.go index 0e207e8..cc4be11 100644 --- a/shared/testutils/testutils.go +++ b/shared/testutils/testutils.go @@ -17,6 +17,7 @@ import ( "github.com/ddworken/hishtory/client/data" "github.com/google/go-cmp/cmp" + "github.com/google/uuid" ) const ( @@ -326,6 +327,7 @@ func MakeFakeHistoryEntry(command string) data.HistoryEntry { StartTime: time.Unix(fakeHistoryTimestamp, 0).UTC(), EndTime: time.Unix(fakeHistoryTimestamp+3, 0).UTC(), DeviceId: "fake_device_id", + EntryId: uuid.Must(uuid.NewRandom()).String(), } } From 9b847c5e35abb0f9bf7f363b5a64caab9ac6467a Mon Sep 17 00:00:00 2001 From: David Dworken Date: Fri, 22 Sep 2023 13:49:29 -0700 Subject: [PATCH 03/12] Further optimize client-server roundtrips by including deletion and dump requests in submit responses (follow up to 1e43de689fa5ee8fb07862cc007c298389670bdb) --- .../server/internal/server/api_handlers.go | 35 ++++------ backend/server/internal/server/server_test.go | 18 +++-- client/cmd/saveHistoryEntry.go | 69 ++++++++----------- client/cmd/status.go | 11 --- client/lib/lib.go | 20 ++---- shared/data.go | 8 +-- 6 files changed, 60 insertions(+), 101 deletions(-) diff --git a/backend/server/internal/server/api_handlers.go b/backend/server/internal/server/api_handlers.go index 5575c0e..037b33e 100644 --- a/backend/server/internal/server/api_handlers.go +++ b/backend/server/internal/server/api_handlers.go @@ -1,7 +1,6 @@ package server import ( - "context" "encoding/json" "fmt" "html" @@ -49,34 +48,26 @@ func (s *Server) apiSubmitHandler(w http.ResponseWriter, r *http.Request) { s.statsd.Count("hishtory.submit", int64(len(devices)), []string{}, 1.0) } + resp := shared.SubmitResponse{} + deviceId := getOptionalQueryParam(r, "source_device_id", s.isTestEnvironment) - resp := shared.SubmitResponse{ - HaveDumpRequests: s.haveDumpRequests(r.Context(), userId, deviceId), - HaveDeletionRequests: s.haveDeletionRequests(r.Context(), userId, deviceId), + if deviceId != "" { + dumpRequests, err := s.db.DumpRequestForUserAndDevice(r.Context(), userId, deviceId) + checkGormError(err) + resp.DumpRequests = dumpRequests + + deletionRequests, err := s.db.DeletionRequestsForUserAndDevice(r.Context(), userId, deviceId) + checkGormError(err) + resp.DeletionRequests = deletionRequests + + // TODO: Update this code to call DeletionRequestInc() iff the version is new enough to be using these responses } + if err := json.NewEncoder(w).Encode(resp); err != nil { panic(err) } } -func (s *Server) haveDumpRequests(ctx context.Context, userId, deviceId string) bool { - if userId == "" || deviceId == "" { - return true - } - dumpRequests, err := s.db.DumpRequestForUserAndDevice(ctx, userId, deviceId) - checkGormError(err) - return len(dumpRequests) > 0 -} - -func (s *Server) haveDeletionRequests(ctx context.Context, userId, deviceId string) bool { - if userId == "" || deviceId == "" { - return true - } - deletionRequests, err := s.db.DeletionRequestsForUserAndDevice(ctx, userId, deviceId) - checkGormError(err) - return len(deletionRequests) > 0 -} - func (s *Server) apiBootstrapHandler(w http.ResponseWriter, r *http.Request) { userId := getRequiredQueryParam(r, "user_id") deviceId := getRequiredQueryParam(r, "device_id") diff --git a/backend/server/internal/server/server_test.go b/backend/server/internal/server/server_test.go index 0f706e6..265c1d0 100644 --- a/backend/server/internal/server/server_test.go +++ b/backend/server/internal/server/server_test.go @@ -81,7 +81,8 @@ func TestESubmitThenQuery(t *testing.T) { w := httptest.NewRecorder() s.apiSubmitHandler(w, submitReq) require.Equal(t, 200, w.Result().StatusCode) - require.Equal(t, shared.SubmitResponse{HaveDumpRequests: true, HaveDeletionRequests: false}, deserializeSubmitResponse(t, w)) + require.Empty(t, deserializeSubmitResponse(t, w).DeletionRequests) + require.NotEmpty(t, deserializeSubmitResponse(t, w).DumpRequests) // Query for device id 1 w = httptest.NewRecorder() @@ -346,7 +347,8 @@ func TestDeletionRequests(t *testing.T) { w := httptest.NewRecorder() s.apiSubmitHandler(w, submitReq) require.Equal(t, 200, w.Result().StatusCode) - require.Equal(t, shared.SubmitResponse{HaveDumpRequests: true, HaveDeletionRequests: false}, deserializeSubmitResponse(t, w)) + require.Empty(t, deserializeSubmitResponse(t, w).DeletionRequests) + require.NotEmpty(t, deserializeSubmitResponse(t, w).DumpRequests) // And another entry for user1 entry2 := testutils.MakeFakeHistoryEntry("ls /foo/bar") @@ -359,7 +361,8 @@ func TestDeletionRequests(t *testing.T) { w = httptest.NewRecorder() s.apiSubmitHandler(w, submitReq) require.Equal(t, 200, w.Result().StatusCode) - require.Equal(t, shared.SubmitResponse{HaveDumpRequests: true, HaveDeletionRequests: false}, deserializeSubmitResponse(t, w)) + require.Empty(t, deserializeSubmitResponse(t, w).DeletionRequests) + require.NotEmpty(t, deserializeSubmitResponse(t, w).DumpRequests) // And an entry for user2 that has the same timestamp as the previous entry entry3 := testutils.MakeFakeHistoryEntry("ls /foo/bar") @@ -373,7 +376,8 @@ func TestDeletionRequests(t *testing.T) { w = httptest.NewRecorder() s.apiSubmitHandler(w, submitReq) require.Equal(t, 200, w.Result().StatusCode) - require.Equal(t, shared.SubmitResponse{HaveDumpRequests: true, HaveDeletionRequests: false}, deserializeSubmitResponse(t, w)) + require.Empty(t, deserializeSubmitResponse(t, w).DeletionRequests) + require.NotEmpty(t, deserializeSubmitResponse(t, w).DumpRequests) // Query for device id 1 w = httptest.NewRecorder() @@ -485,7 +489,8 @@ func TestDeletionRequests(t *testing.T) { w = httptest.NewRecorder() s.apiSubmitHandler(w, submitReq) require.Equal(t, 200, w.Result().StatusCode) - require.Equal(t, shared.SubmitResponse{HaveDumpRequests: true, HaveDeletionRequests: true}, deserializeSubmitResponse(t, w)) + require.NotEmpty(t, deserializeSubmitResponse(t, w).DeletionRequests) + require.NotEmpty(t, deserializeSubmitResponse(t, w).DumpRequests) // Query for deletion requests w = httptest.NewRecorder() @@ -585,7 +590,8 @@ func TestCleanDatabaseNoErrors(t *testing.T) { w := httptest.NewRecorder() s.apiSubmitHandler(w, submitReq) require.Equal(t, 200, w.Result().StatusCode) - require.Equal(t, shared.SubmitResponse{HaveDumpRequests: true, HaveDeletionRequests: false}, deserializeSubmitResponse(t, w)) + require.Empty(t, deserializeSubmitResponse(t, w).DeletionRequests) + require.NotEmpty(t, deserializeSubmitResponse(t, w).DumpRequests) // Call cleanDatabase and just check that there are no panics testutils.Check(t, DB.Clean(context.TODO())) diff --git a/client/cmd/saveHistoryEntry.go b/client/cmd/saveHistoryEntry.go index 56c6fe3..b5248ec 100644 --- a/client/cmd/saveHistoryEntry.go +++ b/client/cmd/saveHistoryEntry.go @@ -187,8 +187,6 @@ func saveHistoryEntry(ctx context.Context) { lib.CheckFatalError(err) // Persist it remotely - shouldCheckForDeletionRequests := true - shouldCheckForDumpRequests := true if !config.IsOffline { jsonValue, err := lib.EncryptAndMarshal(config, []*data.HistoryEntry{entry}) lib.CheckFatalError(err) @@ -200,49 +198,11 @@ func saveHistoryEntry(ctx context.Context) { if err != nil { lib.CheckFatalError(fmt.Errorf("failed to deserialize response from /api/v1/submit: %w", err)) } - shouldCheckForDeletionRequests = submitResponse.HaveDeletionRequests - shouldCheckForDumpRequests = submitResponse.HaveDumpRequests + lib.CheckFatalError(handleDumpRequests(ctx, submitResponse.DumpRequests)) + lib.CheckFatalError(lib.HandleDeletionRequests(ctx, submitResponse.DeletionRequests)) } } - // Check if there is a pending dump request and reply to it if so - if shouldCheckForDumpRequests { - dumpRequests, err := lib.GetDumpRequests(config) - if err != nil { - if lib.IsOfflineError(err) { - // It is fine to just ignore this, the next command will retry the API and eventually we will respond to any pending dump requests - dumpRequests = []*shared.DumpRequest{} - hctx.GetLogger().Infof("Failed to check for dump requests because we failed to connect to the remote server!") - } else { - lib.CheckFatalError(err) - } - } - if len(dumpRequests) > 0 { - lib.CheckFatalError(lib.RetrieveAdditionalEntriesFromRemote(ctx)) - entries, err := lib.Search(ctx, db, "", 0) - lib.CheckFatalError(err) - var encEntries []*shared.EncHistoryEntry - for _, entry := range entries { - enc, err := data.EncryptHistoryEntry(config.UserSecret, *entry) - lib.CheckFatalError(err) - encEntries = append(encEntries, &enc) - } - reqBody, err := json.Marshal(encEntries) - lib.CheckFatalError(err) - for _, dumpRequest := range dumpRequests { - if !config.IsOffline { - _, err := lib.ApiPost("/api/v1/submit-dump?user_id="+dumpRequest.UserId+"&requesting_device_id="+dumpRequest.RequestingDeviceId+"&source_device_id="+config.DeviceId, "application/json", reqBody) - lib.CheckFatalError(err) - } - } - } - } - - // Handle deletion requests - if shouldCheckForDeletionRequests { - lib.CheckFatalError(lib.ProcessDeletionRequests(ctx)) - } - if config.BetaMode { db.Commit() } @@ -253,6 +213,31 @@ func init() { rootCmd.AddCommand(presaveHistoryEntryCmd) } +func handleDumpRequests(ctx context.Context, dumpRequests []*shared.DumpRequest) error { + db := hctx.GetDb(ctx) + config := hctx.GetConf(ctx) + if len(dumpRequests) > 0 { + lib.CheckFatalError(lib.RetrieveAdditionalEntriesFromRemote(ctx)) + entries, err := lib.Search(ctx, db, "", 0) + lib.CheckFatalError(err) + var encEntries []*shared.EncHistoryEntry + for _, entry := range entries { + enc, err := data.EncryptHistoryEntry(config.UserSecret, *entry) + lib.CheckFatalError(err) + encEntries = append(encEntries, &enc) + } + reqBody, err := json.Marshal(encEntries) + lib.CheckFatalError(err) + for _, dumpRequest := range dumpRequests { + if !config.IsOffline { + _, err := lib.ApiPost("/api/v1/submit-dump?user_id="+dumpRequest.UserId+"&requesting_device_id="+dumpRequest.RequestingDeviceId+"&source_device_id="+config.DeviceId, "application/json", reqBody) + lib.CheckFatalError(err) + } + } + } + return nil +} + func buildPreArgsHistoryEntry(ctx context.Context) (*data.HistoryEntry, error) { var entry data.HistoryEntry diff --git a/client/cmd/status.go b/client/cmd/status.go index f598873..cd26c33 100644 --- a/client/cmd/status.go +++ b/client/cmd/status.go @@ -22,7 +22,6 @@ var statusCmd = &cobra.Command{ if *verbose { fmt.Printf("User ID: %s\n", data.UserId(config.UserSecret)) fmt.Printf("Device ID: %s\n", config.DeviceId) - printDumpStatus(config) printOnlineStatus(config) } fmt.Printf("Commit Hash: %s\n", lib.GitCommit) @@ -42,16 +41,6 @@ func printOnlineStatus(config hctx.ClientConfig) { } } -func printDumpStatus(config hctx.ClientConfig) { - dumpRequests, err := lib.GetDumpRequests(config) - lib.CheckFatalError(err) - fmt.Printf("Dump Requests: ") - for _, d := range dumpRequests { - fmt.Printf("%#v, ", *d) - } - fmt.Print("\n") -} - func init() { rootCmd.AddCommand(statusCmd) verbose = statusCmd.Flags().BoolP("verbose", "v", false, "Display verbose hiSHtory information") diff --git a/client/lib/lib.go b/client/lib/lib.go index 41e9523..6d37a9c 100644 --- a/client/lib/lib.go +++ b/client/lib/lib.go @@ -598,6 +598,10 @@ func ProcessDeletionRequests(ctx context.Context) error { if err != nil { return err } + return HandleDeletionRequests(ctx, deletionRequests) +} + +func HandleDeletionRequests(ctx context.Context, deletionRequests []*shared.DeletionRequest) error { db := hctx.GetDb(ctx) for _, request := range deletionRequests { for _, entry := range request.Messages.Ids { @@ -876,22 +880,6 @@ func unescape(query string) string { return string(newQuery) } -func GetDumpRequests(config hctx.ClientConfig) ([]*shared.DumpRequest, error) { - if config.IsOffline { - return make([]*shared.DumpRequest, 0), nil - } - resp, err := ApiGet("/api/v1/get-dump-requests?user_id=" + data.UserId(config.UserSecret) + "&device_id=" + config.DeviceId) - if IsOfflineError(err) { - return []*shared.DumpRequest{}, nil - } - if err != nil { - return nil, err - } - var dumpRequests []*shared.DumpRequest - err = json.Unmarshal(resp, &dumpRequests) - return dumpRequests, err -} - func SendDeletionRequest(deletionRequest shared.DeletionRequest) error { data, err := json.Marshal(deletionRequest) if err != nil { diff --git a/shared/data.go b/shared/data.go index 0c6cc79..b245bdb 100644 --- a/shared/data.go +++ b/shared/data.go @@ -118,11 +118,11 @@ type Feedback struct { Feedback string `json:"feedback"` } -// Response from submitting new history entries. Contains metadata that is used to avoid making additional round-trip -// requests to the hishtory backend. +// Response from submitting new history entries. Contains deletion requests and dump requests to avoid +// extra round-trip requests to the hishtory backend. type SubmitResponse struct { - HaveDumpRequests bool `json:"have_dump_requests"` - HaveDeletionRequests bool `json:"have_deletion_requests"` + DumpRequests []*DumpRequest `json:"dump_requests"` + DeletionRequests []*DeletionRequest `json:"deletion_requests"` } func Chunks[k any](slice []k, chunkSize int) [][]k { From 1d878195b201d6c279abb60c15063f7346c17028 Mon Sep 17 00:00:00 2001 From: David Dworken Date: Fri, 22 Sep 2023 14:03:41 -0700 Subject: [PATCH 04/12] Rework ff98a7907c9d05b4763920f46dec7c4b331b91d1 to use the newly added EntryId column rather than deleting based on the start time --- backend/server/internal/database/db.go | 10 ++-------- client/cmd/redact.go | 2 +- client/lib/lib.go | 6 +++--- client/tui/tui.go | 2 +- shared/data.go | 16 ++++++++++------ 5 files changed, 17 insertions(+), 19 deletions(-) diff --git a/backend/server/internal/database/db.go b/backend/server/internal/database/db.go index 6c6e520..f4ef87f 100644 --- a/backend/server/internal/database/db.go +++ b/backend/server/internal/database/db.go @@ -152,14 +152,8 @@ func (db *DB) DumpRequestDeleteForUserAndDevice(ctx context.Context, userID, dev func (db *DB) ApplyDeletionRequestsToBackend(ctx context.Context, request *shared.DeletionRequest) (int64, error) { tx := db.WithContext(ctx).Where("false") for _, message := range request.Messages.Ids { - // Note that this won't do server-side deletion of pre-saved history entries. This is an inherent - // limitation of our current DB schema. This is sub-par, since it means that even after deletion, clients - // may still receive deleted history entries. But, a well-behaved client will immediately delete - // these (never writing them to disk) and mark them as received, so this won't happen again. - // - // TODO: This could be improved upon if we added a HistoryEntry.EntryId field, backfilled it, added - // it to EncHistoryEntry, and then used that as a key for deletion. - tx = tx.Or(db.WithContext(ctx).Where("user_id = ? AND device_id = ? AND date = ?", request.UserId, message.DeviceId, message.EndTime)) + // Note that we do an OR with date or the ID matching since the ID is not always recorded for older history entries. + tx = tx.Or(db.WithContext(ctx).Where("user_id = ? AND device_id = ? AND (date = ? OR id = ?)", request.UserId, message.DeviceId, message.EndTime, message.EntryId)) } result := tx.Delete(&shared.EncHistoryEntry{}) if tx.Error != nil { diff --git a/client/cmd/redact.go b/client/cmd/redact.go index a79903e..998cd93 100644 --- a/client/cmd/redact.go +++ b/client/cmd/redact.go @@ -87,7 +87,7 @@ func deleteOnRemoteInstances(ctx context.Context, historyEntries []*data.History for _, entry := range historyEntries { deletionRequest.Messages.Ids = append(deletionRequest.Messages.Ids, - shared.MessageIdentifier{StartTime: entry.StartTime, EndTime: entry.EndTime, DeviceId: entry.DeviceId}, + shared.MessageIdentifier{DeviceId: entry.DeviceId, EndTime: entry.EndTime, EntryId: entry.EntryId}, ) } return lib.SendDeletionRequest(deletionRequest) diff --git a/client/lib/lib.go b/client/lib/lib.go index 6d37a9c..817d20a 100644 --- a/client/lib/lib.go +++ b/client/lib/lib.go @@ -605,9 +605,9 @@ func HandleDeletionRequests(ctx context.Context, deletionRequests []*shared.Dele db := hctx.GetDb(ctx) for _, request := range deletionRequests { for _, entry := range request.Messages.Ids { - // Note that entry.StartTime is not always present (for legacy reasons) and entry.EndTime is also - // not always present (for pre-saved entries). So we just check that one of them matches. - tx := db.Where("device_id = ? AND (start_time = ? OR end_time = ?)", entry.DeviceId, entry.StartTime, entry.EndTime) + // Note that entry.EndTime is not always present (for pre-saved entries). And likewise, + // entry.EntryId is not always present for older entries. So we just check that one of them matches. + tx := db.Where("device_id = ? AND (end_time = ? OR entry_id = ?)", entry.DeviceId, entry.EndTime, entry.EntryId) res := tx.Delete(&data.HistoryEntry{}) if res.Error != nil { return fmt.Errorf("DB error: %w", res.Error) diff --git a/client/tui/tui.go b/client/tui/tui.go index ed65dcf..4f81cd1 100644 --- a/client/tui/tui.go +++ b/client/tui/tui.go @@ -602,7 +602,7 @@ func deleteHistoryEntry(ctx context.Context, entry data.HistoryEntry) error { SendTime: time.Now(), } dr.Messages.Ids = append(dr.Messages.Ids, - shared.MessageIdentifier{StartTime: entry.StartTime, EndTime: entry.EndTime, DeviceId: entry.DeviceId}, + shared.MessageIdentifier{DeviceId: entry.DeviceId, EndTime: entry.EndTime, EntryId: entry.EntryId}, ) return lib.SendDeletionRequest(dr) } diff --git a/shared/data.go b/shared/data.go index b245bdb..a48e7bf 100644 --- a/shared/data.go +++ b/shared/data.go @@ -14,13 +14,15 @@ type EncHistoryEntry struct { DeviceId string `json:"device_id"` UserId string `json:"user_id"` // Note that EncHistoryEntry.Date == HistoryEntry.EndTime - Date time.Time `json:"time"` - EncryptedId string `json:"id"` - ReadCount int `json:"read_count"` + Date time.Time `json:"time"` + // Note that EncHistoryEntry.EncryptedId == HistoryEntry.Id (for entries created after pre-saving support) + EncryptedId string `json:"encrypted_id"` + ReadCount int `json:"read_count"` } /* Manually created the indices: +CREATE INDEX CONCURRENTLY entry_id_idx ON enc_history_entries USING btree(encrypted_id); CREATE INDEX CONCURRENTLY device_id_idx ON enc_history_entries USING btree(device_id); CREATE INDEX CONCURRENTLY read_count_idx ON enc_history_entries USING btree(read_count); CREATE INDEX CONCURRENTLY redact_idx ON enc_history_entries USING btree(user_id, device_id, date); @@ -82,7 +84,7 @@ type MessageIdentifiers struct { Ids []MessageIdentifier `json:"message_ids"` } -// Identifies a single history entry based on the device that recorded the entry, and the end time. Note that +// Identifies a single history entry based on the device that recorded the entry, and additional metadata. Note that // this does not include the command itself since that would risk including the sensitive data that is meant // to be deleted type MessageIdentifier struct { @@ -90,9 +92,11 @@ type MessageIdentifier struct { DeviceId string `json:"device_id"` // The timestamp when the command finished running. Serialized as "date" for legacy compatibility. EndTime time.Time `json:"date"` - // The timestamp when the command started running. + // The entry ID of the command. // Note this field was added as part of supporting pre-saving commands, so older clients do not set this field - StartTime time.Time `json:"start_time"` + // And even for new clients, it may contain a per-device entry ID. For pre-saved entries, this is guaranteed to + // be present. + EntryId string `json:"entry_id"` } func (m *MessageIdentifiers) Scan(value interface{}) error { From 8c6443ed071312f89b93e11456ad091d0e40a8bf Mon Sep 17 00:00:00 2001 From: David Dworken Date: Fri, 22 Sep 2023 18:59:37 -0700 Subject: [PATCH 05/12] Fix incorrect column name --- backend/server/internal/database/db.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/server/internal/database/db.go b/backend/server/internal/database/db.go index f4ef87f..19b9b98 100644 --- a/backend/server/internal/database/db.go +++ b/backend/server/internal/database/db.go @@ -153,7 +153,7 @@ func (db *DB) ApplyDeletionRequestsToBackend(ctx context.Context, request *share tx := db.WithContext(ctx).Where("false") for _, message := range request.Messages.Ids { // Note that we do an OR with date or the ID matching since the ID is not always recorded for older history entries. - tx = tx.Or(db.WithContext(ctx).Where("user_id = ? AND device_id = ? AND (date = ? OR id = ?)", request.UserId, message.DeviceId, message.EndTime, message.EntryId)) + tx = tx.Or(db.WithContext(ctx).Where("user_id = ? AND device_id = ? AND (date = ? OR encrypted_id = ?)", request.UserId, message.DeviceId, message.EndTime, message.EntryId)) } result := tx.Delete(&shared.EncHistoryEntry{}) if tx.Error != nil { From 3f0adbc3241fe2740ff85d0d56fe91f7ef3743cc Mon Sep 17 00:00:00 2001 From: David Dworken Date: Fri, 22 Sep 2023 19:00:37 -0700 Subject: [PATCH 06/12] Add support for deleting pre-saved entries on the remote server --- client/cmd/saveHistoryEntry.go | 71 ++++++++++++++++++++++++---------- 1 file changed, 51 insertions(+), 20 deletions(-) diff --git a/client/cmd/saveHistoryEntry.go b/client/cmd/saveHistoryEntry.go index b5248ec..31c25e9 100644 --- a/client/cmd/saveHistoryEntry.go +++ b/client/cmd/saveHistoryEntry.go @@ -20,6 +20,7 @@ import ( "github.com/ddworken/hishtory/shared" "github.com/google/uuid" "github.com/spf13/cobra" + "gorm.io/gorm" ) var saveHistoryEntryCmd = &cobra.Command{ @@ -161,25 +162,7 @@ func saveHistoryEntry(ctx context.Context) { // Drop any entries from pre-saving since they're no longer needed if config.BetaMode { - deletePresavedEntryFunc := func() error { - query := "cwd:" + entry.CurrentWorkingDirectory - query += " start_time:" + strconv.FormatInt(entry.StartTime.Unix(), 10) - query += " end_time:1970/01/01_00:00:00_+00:00" - tx, err := lib.MakeWhereQueryFromSearch(ctx, db, query) - if err != nil { - return fmt.Errorf("failed to query for pre-saved history entry: %w", err) - } - tx.Where("command = ?", entry.Command) - res := tx.Delete(&data.HistoryEntry{}) - if res.Error != nil { - return fmt.Errorf("failed to delete pre-saved history entry (expected command=%#v): %w", entry.Command, res.Error) - } - if res.RowsAffected > 1 { - return fmt.Errorf("attempted to delete pre-saved entry, but something went wrong since we deleted %d rows", res.RowsAffected) - } - return nil - } - lib.CheckFatalError(lib.RetryingDbFunction(deletePresavedEntryFunc)) + lib.CheckFatalError(deletePresavedEntries(ctx, entry)) } // Persist it locally @@ -198,8 +181,8 @@ func saveHistoryEntry(ctx context.Context) { if err != nil { lib.CheckFatalError(fmt.Errorf("failed to deserialize response from /api/v1/submit: %w", err)) } - lib.CheckFatalError(handleDumpRequests(ctx, submitResponse.DumpRequests)) lib.CheckFatalError(lib.HandleDeletionRequests(ctx, submitResponse.DeletionRequests)) + lib.CheckFatalError(handleDumpRequests(ctx, submitResponse.DumpRequests)) } } @@ -208,6 +191,54 @@ func saveHistoryEntry(ctx context.Context) { } } +func deletePresavedEntries(ctx context.Context, entry *data.HistoryEntry) error { + db := hctx.GetDb(ctx) + + // Create the query to find the presaved entries + query := "cwd:" + entry.CurrentWorkingDirectory + query += " start_time:" + strconv.FormatInt(entry.StartTime.Unix(), 10) + query += " end_time:1970/01/01_00:00:00_+00:00" + matchingEntryQuery, err := lib.MakeWhereQueryFromSearch(ctx, db, query) + if err != nil { + return fmt.Errorf("failed to query for pre-saved history entry: %w", err) + } + matchingEntryQuery = matchingEntryQuery.Where("command = ?", entry.Command).Session(&gorm.Session{}) + + // Get the presaved entry since we need it for doing remote deletes + var presavedEntry data.HistoryEntry + res := matchingEntryQuery.Find(&presavedEntry) + if res.Error != nil { + return fmt.Errorf("failed to search for presaved entry for cmd=%#v: %w", entry.Command, res.Error) + } + + // Delete presaved entries locally + deletePresavedEntryFunc := func() error { + res := matchingEntryQuery.Delete(&data.HistoryEntry{}) + if res.Error != nil { + return fmt.Errorf("failed to delete pre-saved history entry (expected command=%#v): %w", entry.Command, res.Error) + } + if res.RowsAffected > 1 { + return fmt.Errorf("attempted to delete pre-saved entry, but something went wrong since we deleted %d rows", res.RowsAffected) + } + return nil + } + err = lib.RetryingDbFunction(deletePresavedEntryFunc) + if err != nil { + return err + } + + // And delete it remotely + config := hctx.GetConf(ctx) + var deletionRequest shared.DeletionRequest + deletionRequest.SendTime = time.Now() + deletionRequest.UserId = data.UserId(config.UserSecret) + deletionRequest.Messages.Ids = append(deletionRequest.Messages.Ids, + // Note that we aren't specifying an EndTime here since pre-saved entries don't have an EndTime + shared.MessageIdentifier{DeviceId: presavedEntry.DeviceId, EntryId: presavedEntry.EntryId}, + ) + return lib.SendDeletionRequest(deletionRequest) +} + func init() { rootCmd.AddCommand(saveHistoryEntryCmd) rootCmd.AddCommand(presaveHistoryEntryCmd) From 51f2c15f35b714739f6a086ad9731b58b3feeaba Mon Sep 17 00:00:00 2001 From: David Dworken Date: Fri, 22 Sep 2023 19:01:27 -0700 Subject: [PATCH 07/12] Add index for deletion requests now that the volume of deletion requests will increase --- shared/data.go | 1 + 1 file changed, 1 insertion(+) diff --git a/shared/data.go b/shared/data.go index a48e7bf..eb2bc66 100644 --- a/shared/data.go +++ b/shared/data.go @@ -26,6 +26,7 @@ CREATE INDEX CONCURRENTLY entry_id_idx ON enc_history_entries USING btree(encryp CREATE INDEX CONCURRENTLY device_id_idx ON enc_history_entries USING btree(device_id); CREATE INDEX CONCURRENTLY read_count_idx ON enc_history_entries USING btree(read_count); CREATE INDEX CONCURRENTLY redact_idx ON enc_history_entries USING btree(user_id, device_id, date); +CREATE INDEX CONCURRENTLY del_user_idx ON deletion_requests USING btree(user_id); */ type Device struct { From cc11916f3cf9b60a5a41847417aa4c91dba4d41a Mon Sep 17 00:00:00 2001 From: David Dworken Date: Fri, 22 Sep 2023 19:11:43 -0700 Subject: [PATCH 08/12] Create func to automatically create DB indexes rather than just documenting them in a comment that has to be manually executed --- backend/server/internal/database/db.go | 20 ++++++++++++++++++++ backend/server/server.go | 4 ++++ shared/data.go | 9 --------- 3 files changed, 24 insertions(+), 9 deletions(-) diff --git a/backend/server/internal/database/db.go b/backend/server/internal/database/db.go index 19b9b98..913dcea 100644 --- a/backend/server/internal/database/db.go +++ b/backend/server/internal/database/db.go @@ -63,6 +63,26 @@ func (db *DB) AddDatabaseTables() error { return nil } +func (db *DB) CreateIndices() error { + // Note: If adding a new index here, consider manually running it on the prod DB using CONCURRENTLY to + // make server startup non-blocking. The benefit of this function is primarily for other people so they + // don't have to manually create these indexes. + var indices = []string{ + `CREATE INDEX IF NOT EXISTS entry_id_idx ON enc_history_entries USING btree(encrypted_id);`, + `CREATE INDEX IF NOT EXISTS device_id_idx ON enc_history_entries USING btree(device_id);`, + `CREATE INDEX IF NOT EXISTS read_count_idx ON enc_history_entries USING btree(read_count);`, + `CREATE INDEX IF NOT EXISTS redact_idx ON enc_history_entries USING btree(user_id, device_id, date);`, + `CREATE INDEX IF NOT EXISTS del_user_idx ON deletion_requests USING btree(user_id);`, + } + for _, index := range indices { + r := db.Exec(index) + if r.Error != nil { + return fmt.Errorf("failed to execute index creation sql=%#v: %w", index, r.Error) + } + } + return nil +} + func (db *DB) Close() error { rawDB, err := db.DB.DB() if err != nil { diff --git a/backend/server/server.go b/backend/server/server.go index 9210955..272b8dc 100644 --- a/backend/server/server.go +++ b/backend/server/server.go @@ -95,6 +95,10 @@ func OpenDB() (*database.DB, error) { return nil, fmt.Errorf("failed to create underlying DB tables: %w", err) } } + err := db.CreateIndices() + if err != nil { + return nil, err + } return db, nil } diff --git a/shared/data.go b/shared/data.go index eb2bc66..2b5f8f4 100644 --- a/shared/data.go +++ b/shared/data.go @@ -20,15 +20,6 @@ type EncHistoryEntry struct { ReadCount int `json:"read_count"` } -/* -Manually created the indices: -CREATE INDEX CONCURRENTLY entry_id_idx ON enc_history_entries USING btree(encrypted_id); -CREATE INDEX CONCURRENTLY device_id_idx ON enc_history_entries USING btree(device_id); -CREATE INDEX CONCURRENTLY read_count_idx ON enc_history_entries USING btree(read_count); -CREATE INDEX CONCURRENTLY redact_idx ON enc_history_entries USING btree(user_id, device_id, date); -CREATE INDEX CONCURRENTLY del_user_idx ON deletion_requests USING btree(user_id); -*/ - type Device struct { UserId string `json:"user_id"` DeviceId string `json:"device_id"` From 5bdbd9b2626a2581b5052bd1ef1076fc3d866d00 Mon Sep 17 00:00:00 2001 From: David Dworken Date: Fri, 22 Sep 2023 19:19:06 -0700 Subject: [PATCH 09/12] Revert increased read_count requirement for deletion requests since deleting via encrypted-ID should be reliable --- backend/server/internal/database/db.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/server/internal/database/db.go b/backend/server/internal/database/db.go index 913dcea..b9804bf 100644 --- a/backend/server/internal/database/db.go +++ b/backend/server/internal/database/db.go @@ -247,7 +247,7 @@ func (db *DB) Clean(ctx context.Context) error { if r.Error != nil { return r.Error } - r = db.WithContext(ctx).Exec("DELETE FROM deletion_requests WHERE read_count > 1000") + r = db.WithContext(ctx).Exec("DELETE FROM deletion_requests WHERE read_count > 100") if r.Error != nil { return r.Error } From bbfda0be96d097001fc42b435fb8f26f6e5b96aa Mon Sep 17 00:00:00 2001 From: David Dworken Date: Fri, 22 Sep 2023 19:20:51 -0700 Subject: [PATCH 10/12] Remove TODO: I think this isn't worth implementing since in the long term, clients will update and this branch will not be necessary (and thus will primarily be tech debt) --- backend/server/internal/server/api_handlers.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/backend/server/internal/server/api_handlers.go b/backend/server/internal/server/api_handlers.go index 037b33e..d869161 100644 --- a/backend/server/internal/server/api_handlers.go +++ b/backend/server/internal/server/api_handlers.go @@ -59,8 +59,6 @@ func (s *Server) apiSubmitHandler(w http.ResponseWriter, r *http.Request) { deletionRequests, err := s.db.DeletionRequestsForUserAndDevice(r.Context(), userId, deviceId) checkGormError(err) resp.DeletionRequests = deletionRequests - - // TODO: Update this code to call DeletionRequestInc() iff the version is new enough to be using these responses } if err := json.NewEncoder(w).Encode(resp); err != nil { From e089690cbb878ac8d38c1cbf17d9d3760cc2f7cf Mon Sep 17 00:00:00 2001 From: David Dworken Date: Fri, 22 Sep 2023 19:22:03 -0700 Subject: [PATCH 11/12] no-op formatting change --- client/cmd/install.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/client/cmd/install.go b/client/cmd/install.go index 5eb5def..90243dd 100644 --- a/client/cmd/install.go +++ b/client/cmd/install.go @@ -181,9 +181,7 @@ func install(secretKey string, offline bool) error { // Handles people running `hishtory update` when the DB needs updating. func handleDbUpgrades(ctx context.Context) error { db := hctx.GetDb(ctx) - return db.Exec(` - UPDATE history_entries SET entry_id = lower(hex(randomblob(12))) WHERE entry_id IS NULL - `).Error + return db.Exec(`UPDATE history_entries SET entry_id = lower(hex(randomblob(12))) WHERE entry_id IS NULL`).Error } // Handles people running `hishtory update` from an old version of hishtory that From 2c9aa099d211fb2ad9488765a04ced8599cd90d1 Mon Sep 17 00:00:00 2001 From: David Dworken Date: Fri, 22 Sep 2023 19:31:22 -0700 Subject: [PATCH 12/12] Fix bug in offline sync code that contained off-by-one error leading to missed entries --- client/cmd/saveHistoryEntry.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/client/cmd/saveHistoryEntry.go b/client/cmd/saveHistoryEntry.go index 31c25e9..e454352 100644 --- a/client/cmd/saveHistoryEntry.go +++ b/client/cmd/saveHistoryEntry.go @@ -57,7 +57,6 @@ func maybeUploadSkippedHistoryEntries(ctx context.Context) error { // Upload the missing entries db := hctx.GetDb(ctx) - // TODO: There is a bug here because MissedUploadTimestamp is going to be a second or two after the history entry that needs to be uploaded query := fmt.Sprintf("after:%s", time.Unix(config.MissedUploadTimestamp, 0).Format("2006-01-02")) entries, err := lib.Search(ctx, db, query, 0) if err != nil { @@ -84,13 +83,15 @@ func maybeUploadSkippedHistoryEntries(ctx context.Context) error { return nil } -func handlePotentialUploadFailure(err error, config *hctx.ClientConfig) { +func handlePotentialUploadFailure(err error, config *hctx.ClientConfig, entryTimestamp time.Time) { if err != nil { if lib.IsOfflineError(err) { hctx.GetLogger().Infof("Failed to remotely persist hishtory entry because we failed to connect to the remote server! This is likely because the device is offline, but also could be because the remote server is having reliability issues. Original error: %v", err) if !config.HaveMissedUploads { config.HaveMissedUploads = true - config.MissedUploadTimestamp = time.Now().Unix() + // Set MissedUploadTimestamp to `entry timestamp - 1` so that the current entry will get + // uploaded once network access is regained. + config.MissedUploadTimestamp = entryTimestamp.UTC().Unix() - 1 lib.CheckFatalError(hctx.SetConfig(*config)) } } else { @@ -142,7 +143,7 @@ func presaveHistoryEntry(ctx context.Context) { jsonValue, err := lib.EncryptAndMarshal(config, []*data.HistoryEntry{entry}) lib.CheckFatalError(err) _, err = lib.ApiPost("/api/v1/submit?source_device_id="+config.DeviceId, "application/json", jsonValue) - handlePotentialUploadFailure(err, &config) + handlePotentialUploadFailure(err, &config, entry.StartTime) } } @@ -174,7 +175,7 @@ func saveHistoryEntry(ctx context.Context) { jsonValue, err := lib.EncryptAndMarshal(config, []*data.HistoryEntry{entry}) lib.CheckFatalError(err) w, err := lib.ApiPost("/api/v1/submit?source_device_id="+config.DeviceId, "application/json", jsonValue) - handlePotentialUploadFailure(err, &config) + handlePotentialUploadFailure(err, &config, entry.StartTime) if err == nil { submitResponse := shared.SubmitResponse{} err := json.Unmarshal(w, &submitResponse)