From 24e76b4abc75dedecfda92deee4c7b7c7516940b Mon Sep 17 00:00:00 2001 From: Paul Fitzpatrick Date: Fri, 18 Dec 2020 12:37:16 -0500 Subject: [PATCH] (core) add endpoints for clearing snapshots and actions Summary: This adds a snapshots/remove and states/remove endpoint, primarily for maintenance work rather than for the end user. If some secret gets into document history, it is useful to be able to purge it in an orderly way. Test Plan: added tests Reviewers: dsagal Reviewed By: dsagal Differential Revision: https://phab.getgrist.com/D2694 --- app/common/UserAPI.ts | 19 +++++++++-- app/server/lib/ActionHistory.ts | 9 ++++++ app/server/lib/ActiveDoc.ts | 18 +++++++++-- app/server/lib/Authorizer.ts | 6 ++-- app/server/lib/DocApi.ts | 44 ++++++++++++++++++++++++-- app/server/lib/DocSnapshots.ts | 21 ++++++++---- app/server/lib/DocStorageManager.ts | 6 +++- app/server/lib/HostedStorageManager.ts | 12 +++++-- app/server/lib/IDocStorageManager.ts | 6 ++-- app/server/lib/requestUtils.ts | 14 ++++++-- 10 files changed, 128 insertions(+), 27 deletions(-) diff --git a/app/common/UserAPI.ts b/app/common/UserAPI.ts index 90e3b3ed..a6a08a29 100644 --- a/app/common/UserAPI.ts +++ b/app/common/UserAPI.ts @@ -306,7 +306,12 @@ export interface DocAPI { addRows(tableId: string, additions: BulkColValues): Promise; removeRows(tableId: string, removals: number[]): Promise; replace(source: DocReplacementOptions): Promise; - getSnapshots(): Promise; + // Get list of document versions (specify raw to bypass caching, which should only make + // a difference if snapshots have "leaked") + getSnapshots(raw?: boolean): Promise; + // remove selected snapshots, or all snapshots that have "leaked" from inventory (should + // be empty), or all but the current snapshot. + removeSnapshots(snapshotIds: string[] | 'unlisted' | 'past'): Promise<{snapshotIds: string[]}>; forceReload(): Promise; recover(recoveryMode: boolean): Promise; // Compare two documents, optionally including details of the changes. @@ -706,8 +711,16 @@ export class DocAPIImpl extends BaseAPI implements DocAPI { }); } - public async getSnapshots(): Promise { - return this.requestJson(`${this._url}/snapshots`); + public async getSnapshots(raw?: boolean): Promise { + return this.requestJson(`${this._url}/snapshots?raw=${raw}`); + } + + public async removeSnapshots(snapshotIds: string[] | 'unlisted' | 'past') { + const body = typeof snapshotIds === 'string' ? { select: snapshotIds } : { snapshotIds }; + return await this.requestJson(`${this._url}/snapshots/remove`, { + method: 'POST', + body: JSON.stringify(body) + }); } public async forceReload(): Promise { diff --git a/app/server/lib/ActionHistory.ts b/app/server/lib/ActionHistory.ts index b324648c..8b162d18 100644 --- a/app/server/lib/ActionHistory.ts +++ b/app/server/lib/ActionHistory.ts @@ -135,6 +135,15 @@ export abstract class ActionHistory { /** Check for any client associated with an action, identified by checksum */ public abstract getActionClientId(actionHash: string): string | undefined; + + /** + * Remove all stored actions except the last keepN and run the VACUUM command + * to reduce the size of the SQLite file. + * + * @param {Int} keepN - The number of most recent actions to keep. The value must be at least 1, and + * will default to 1 if not given. + */ + public abstract deleteActions(keepN: number): Promise; } diff --git a/app/server/lib/ActiveDoc.ts b/app/server/lib/ActiveDoc.ts index 831fc31b..0725b154 100644 --- a/app/server/lib/ActiveDoc.ts +++ b/app/server/lib/ActiveDoc.ts @@ -979,9 +979,23 @@ export class ActiveDoc extends EventEmitter { this._inactivityTimer.ping(); } - public async getSnapshots(): Promise { + public async getSnapshots(skipMetadataCache?: boolean): Promise { // Assume any viewer can access this list. - return this._docManager.storageManager.getSnapshots(this.docName); + return this._docManager.storageManager.getSnapshots(this.docName, skipMetadataCache); + } + + public async removeSnapshots(docSession: OptDocSession, snapshotIds: string[]): Promise { + if (!this.isOwner(docSession)) { + throw new Error('cannot remove snapshots, access denied'); + } + return this._docManager.storageManager.removeSnapshots(this.docName, snapshotIds); + } + + public async deleteActions(docSession: OptDocSession, keepN: number): Promise { + if (!this.isOwner(docSession)) { + throw new Error('cannot delete actions, access denied'); + } + this._actionHistory.deleteActions(keepN); } /** diff --git a/app/server/lib/Authorizer.ts b/app/server/lib/Authorizer.ts index 744c36c5..0f339688 100644 --- a/app/server/lib/Authorizer.ts +++ b/app/server/lib/Authorizer.ts @@ -371,7 +371,7 @@ export interface Authorizer { getDoc(): Promise; // Check access, throw error if the requested level of access isn't available. - assertAccess(role: 'viewers'|'editors'): Promise; + assertAccess(role: 'viewers'|'editors'|'owners'): Promise; // Get the lasted access information calculated for the doc. This is useful // for logging - but access control itself should use assertAccess() to @@ -416,7 +416,7 @@ export class DocAuthorizer implements Authorizer { return this._dbManager.getDoc(this._key); } - public async assertAccess(role: 'viewers'|'editors'): Promise { + public async assertAccess(role: 'viewers'|'editors'|'owners'): Promise { const docAuth = await this._dbManager.getDocAuthCached(this._key); this._docAuth = docAuth; assertAccess(role, docAuth, {openMode: this.openMode}); @@ -447,7 +447,7 @@ export class DummyAuthorizer implements Authorizer { export function assertAccess( - role: 'viewers'|'editors', docAuth: DocAuthResult, options: { + role: 'viewers'|'editors'|'owners', docAuth: DocAuthResult, options: { openMode?: OpenDocMode, allowRemoved?: boolean, } = {}) { diff --git a/app/server/lib/DocApi.ts b/app/server/lib/DocApi.ts index 910bf5c6..15039597 100644 --- a/app/server/lib/DocApi.ts +++ b/app/server/lib/DocApi.ts @@ -18,7 +18,8 @@ import { expressWrap } from 'app/server/lib/expressWrap'; import { GristServer } from 'app/server/lib/GristServer'; import { HashUtil } from 'app/server/lib/HashUtil'; import { makeForkIds } from "app/server/lib/idUtils"; -import { getDocId, getDocScope, integerParam, isParameterOn, optStringParam, +import { + getDocId, getDocScope, integerParam, isParameterOn, optStringParam, sendOkReply, sendReply, stringParam } from 'app/server/lib/requestUtils'; import { SandboxError } from "app/server/lib/sandboxUtil"; import { handleOptionalUpload, handleUpload } from "app/server/lib/uploads"; @@ -85,6 +86,7 @@ export class DocWorkerApi { const canView = expressWrap(this._assertAccess.bind(this, 'viewers', false)); // check document exists (not soft deleted) and user can edit it const canEdit = expressWrap(this._assertAccess.bind(this, 'editors', false)); + const isOwner = expressWrap(this._assertAccess.bind(this, 'owners', false)); // check user can edit document, with soft-deleted documents being acceptable const canEditMaybeRemoved = expressWrap(this._assertAccess.bind(this, 'editors', true)); // check document exists, don't check user access @@ -252,10 +254,40 @@ export class DocWorkerApi { })); this._app.get('/api/docs/:docId/snapshots', canView, withDoc(async (activeDoc, req, res) => { - const {snapshots} = await activeDoc.getSnapshots(); + const {snapshots} = await activeDoc.getSnapshots(isAffirmative(req.query.raw)); res.json({snapshots}); })); + this._app.post('/api/docs/:docId/snapshots/remove', isOwner, withDoc(async (activeDoc, req, res) => { + const docSession = docSessionFromRequest(req); + const snapshotIds = req.body.snapshotIds as string[]; + if (snapshotIds) { + await activeDoc.removeSnapshots(docSession, snapshotIds); + res.json({snapshotIds}); + return; + } + if (req.body.select === 'unlisted') { + // Remove any snapshots not listed in inventory. Ideally, there should be no + // snapshots, and this undocument feature is just for fixing up problems. + const full = (await activeDoc.getSnapshots(true)).snapshots.map(s => s.snapshotId); + const listed = new Set((await activeDoc.getSnapshots()).snapshots.map(s => s.snapshotId)); + const unlisted = full.filter(snapshotId => !listed.has(snapshotId)); + await activeDoc.removeSnapshots(docSession, unlisted); + res.json({snapshotIds: unlisted}); + return; + } + if (req.body.select === 'past') { + // Remove all but the latest snapshot. Useful for sanitizing history if something + // bad snuck into previous snapshots and they are not valuable to preserve. + const past = (await activeDoc.getSnapshots(true)).snapshots.map(s => s.snapshotId); + past.shift(); // remove current version. + await activeDoc.removeSnapshots(docSession, past); + res.json({snapshotIds: past}); + return; + } + throw new Error('please specify snapshotIds to remove'); + })); + this._app.post('/api/docs/:docId/flush', canEdit, throttled(async (req, res) => { const activeDocPromise = this._getActiveDocIfAvailable(req); if (!activeDocPromise) { @@ -320,6 +352,12 @@ export class DocWorkerApi { res.json(await this._getStates(docSession, activeDoc)); })); + this._app.post('/api/docs/:docId/states/remove', isOwner, withDoc(async (activeDoc, req, res) => { + const docSession = docSessionFromRequest(req); + const keep = integerParam(req.body.keep); + res.json(await activeDoc.deleteActions(docSession, keep)); + })); + this._app.get('/api/docs/:docId/compare/:docId2', canView, withDoc(async (activeDoc, req, res) => { const showDetails = isAffirmative(req.query.detail); const docSession = docSessionFromRequest(req); @@ -453,7 +491,7 @@ export class DocWorkerApi { return this._docManager.getActiveDoc(getDocId(req)); } - private async _assertAccess(role: 'viewers'|'editors'|null, allowRemoved: boolean, + private async _assertAccess(role: 'viewers'|'editors'|'owners'|null, allowRemoved: boolean, req: Request, res: Response, next: NextFunction) { const scope = getDocScope(req); allowRemoved = scope.showAll || scope.showRemoved || allowRemoved; diff --git a/app/server/lib/DocSnapshots.ts b/app/server/lib/DocSnapshots.ts index cb80f91e..2b16cf3c 100644 --- a/app/server/lib/DocSnapshots.ts +++ b/app/server/lib/DocSnapshots.ts @@ -4,7 +4,7 @@ import { KeyedMutex } from 'app/common/KeyedMutex'; import { ExternalStorage } from 'app/server/lib/ExternalStorage'; import * as log from 'app/server/lib/log'; import * as fse from 'fs-extra'; -import * as moment from 'moment'; +import * as moment from 'moment-timezone'; /** * A subset of the ExternalStorage interface, focusing on maintaining a list of versions. @@ -63,12 +63,19 @@ export class DocSnapshotPruner { return shouldKeepSnapshots(versions).map((keep, index) => ({keep, snapshot: versions[index]})); } - // Prune the specified document immediately. - public async prune(key: string) { - const versions = await this.classify(key); - const redundant = versions.filter(v => !v.keep); - await this._ext.remove(key, redundant.map(r => r.snapshot.snapshotId)); - log.info(`Pruned ${redundant.length} versions of ${versions.length} for document ${key}`); + // Prune the specified document immediately. If no snapshotIds are provided, they + // will be chosen automatically. + public async prune(key: string, snapshotIds?: string[]) { + if (!snapshotIds) { + const versions = await this.classify(key); + const redundant = versions.filter(v => !v.keep); + snapshotIds = redundant.map(r => r.snapshot.snapshotId); + await this._ext.remove(key, snapshotIds); + log.info(`Pruned ${snapshotIds.length} versions of ${versions.length} for document ${key}`); + } else { + await this._ext.remove(key, snapshotIds); + log.info(`Pruned ${snapshotIds.length} externally selected versions for document ${key}`); + } } } diff --git a/app/server/lib/DocStorageManager.ts b/app/server/lib/DocStorageManager.ts index 7c41e8f3..9e6eade5 100644 --- a/app/server/lib/DocStorageManager.ts +++ b/app/server/lib/DocStorageManager.ts @@ -232,10 +232,14 @@ export class DocStorageManager implements IDocStorageManager { return tmpPath; } - public async getSnapshots(docName: string): Promise { + public async getSnapshots(docName: string, skipMetadataCache?: boolean): Promise { throw new Error('getSnapshots not implemented'); } + public removeSnapshots(docName: string, snapshotIds: string[]): Promise { + throw new Error('removeSnapshots not implemented'); + } + public async replace(docName: string, options: any): Promise { throw new Error('replacement not implemented'); } diff --git a/app/server/lib/HostedStorageManager.ts b/app/server/lib/HostedStorageManager.ts index c2873b22..ec217c9d 100644 --- a/app/server/lib/HostedStorageManager.ts +++ b/app/server/lib/HostedStorageManager.ts @@ -454,7 +454,12 @@ export class HostedStorageManager implements IDocStorageManager { return this._uploads.hasPendingOperations(); } - public async getSnapshots(docName: string): Promise { + public async removeSnapshots(docName: string, snapshotIds: string[]): Promise { + if (this._disableS3) { return; } + await this._pruner.prune(docName, snapshotIds); + } + + public async getSnapshots(docName: string, skipMetadataCache?: boolean): Promise { if (this._disableS3) { return { snapshots: [{ @@ -464,8 +469,9 @@ export class HostedStorageManager implements IDocStorageManager { }] }; } - const versions = await this._inventory.versions(docName, - this._latestVersions.get(docName) || null); + const versions = skipMetadataCache ? + await this._ext.versions(docName) : + await this._inventory.versions(docName, this._latestVersions.get(docName) || null); const parts = parseUrlId(docName); return { snapshots: versions diff --git a/app/server/lib/IDocStorageManager.ts b/app/server/lib/IDocStorageManager.ts index 56560a84..fe708919 100644 --- a/app/server/lib/IDocStorageManager.ts +++ b/app/server/lib/IDocStorageManager.ts @@ -29,7 +29,9 @@ export interface IDocStorageManager { getCopy(docName: string): Promise; // get an immutable copy of a document flushDoc(docName: string): Promise; // flush a document to persistent storage - - getSnapshots(docName: string): Promise; + // If skipMetadataCache is set, then any caching of snapshots lists should be skipped. + // Metadata may not be returned in this case. + getSnapshots(docName: string, skipMetadataCache?: boolean): Promise; + removeSnapshots(docName: string, snapshotIds: string[]): Promise; replace(docName: string, options: DocReplacementOptions): Promise; } diff --git a/app/server/lib/requestUtils.ts b/app/server/lib/requestUtils.ts index 03214add..e82bd4fc 100644 --- a/app/server/lib/requestUtils.ts +++ b/app/server/lib/requestUtils.ts @@ -213,9 +213,10 @@ export function optStringParam(p: any): string|undefined { return undefined; } -export function stringParam(p: any): string { - if (typeof p === 'string') { return p; } - throw new Error(`parameter should be a string: ${p}`); +export function stringParam(p: any, allowed?: string[]): string { + if (typeof p !== 'string') { throw new Error(`parameter should be a string: ${p}`); } + if (allowed && !allowed.includes(p)) { throw new Error(`parameter ${p} should be one of ${allowed}`); } + return p; } export function integerParam(p: any): number { @@ -224,6 +225,13 @@ export function integerParam(p: any): number { throw new Error(`parameter should be an integer: ${p}`); } +export function optIntegerParam(p: any): number|undefined { + if (typeof p === 'number') { return Math.floor(p); } + if (typeof p === 'string') { return parseInt(p, 10); } + return undefined; +} + + export interface RequestWithGristInfo extends Request { gristInfo?: string; }