From 699df7b3d17f9311b77525e057aba13973431a11 Mon Sep 17 00:00:00 2001 From: jaegeral Date: Mon, 22 Jun 2026 13:21:51 +0000 Subject: [PATCH 01/11] add test case and fix --- .../timesketch_cli_client/commands/sketch.py | 11 +++++- end_to_end_tests/cli_client_e2e_test.py | 35 +++++++++++++++++++ 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/cli_client/python/timesketch_cli_client/commands/sketch.py b/cli_client/python/timesketch_cli_client/commands/sketch.py index 7235b71248..c0f3ad3d98 100644 --- a/cli_client/python/timesketch_cli_client/commands/sketch.py +++ b/cli_client/python/timesketch_cli_client/commands/sketch.py @@ -361,8 +361,17 @@ def delete_sketch(ctx: click.Context, force_delete: bool) -> None: ) for timeline in sketch.list_timelines(): + try: + # timeline.description and timeline.status lazy-load from the API. + # If the sketch is already soft-deleted, the timeline endpoint + # returns a 404, which raises a RuntimeError. + timeline_desc = timeline.description + timeline_status = timeline.status + except RuntimeError: + timeline_desc = "N/A" + timeline_status = "N/A" click.echo( - f" Timeline: {timeline.id} {timeline.name} {timeline.description} {timeline.status}" # pylint: disable=line-too-long + f" Timeline: {timeline.id} {timeline.name} {timeline_desc} {timeline_status}" # pylint: disable=line-too-long ) if force_delete: diff --git a/end_to_end_tests/cli_client_e2e_test.py b/end_to_end_tests/cli_client_e2e_test.py index 715c591873..2847782a4f 100644 --- a/end_to_end_tests/cli_client_e2e_test.py +++ b/end_to_end_tests/cli_client_e2e_test.py @@ -215,6 +215,41 @@ def test_cli_integration(self): self.assertions.assertEqual(result.exit_code, 0, f"Failed: {result.output}") self.assertions.assertEqual(result.output.strip(), "42") + def test_cli_sketch_delete_soft_deleted(self): + """Tests that a soft-deleted sketch can be deleted via CLI.""" + # Create a new sketch to be soft-deleted and then hard-deleted + sketch_name = f"cli_soft_delete_test_{uuid.uuid4().hex}" + sketch = self.api.create_sketch(name=sketch_name) + + # We need a timeline to trigger the loop in sketch.py + self.import_timeline("evtx_part.csv", sketch=sketch) + + # Soft delete the sketch + sketch.delete(force_delete=False) + + # Now try to delete it via CLI (dry-run first, then force) + cli_ctx_obj = E2ECliContextObject( + api_client=self.api, + sketch_instance=sketch, + output_format="text", + ) + + # Dry-run + result = self.runner.invoke(sketch_group, ["delete"], obj=cli_ctx_obj) + self.assertions.assertEqual( + result.exit_code, + 0, + f"CLI command 'sketch delete' (dry-run) failed on soft-deleted sketch.\nOutput:\n{result.output}\nException:\n{result.exception}" + ) + + # Force-delete + result_force = self.runner.invoke(sketch_group, ["delete", "--force_delete"], obj=cli_ctx_obj) + self.assertions.assertEqual( + result_force.exit_code, + 0, + f"CLI command 'sketch delete --force_delete' failed on soft-deleted sketch.\nOutput:\n{result_force.output}\nException:\n{result_force.exception}" + ) + # Register the new test class with the test manager manager.EndToEndTestManager.register_test(CliClientE2ETest) From 6be9822d747bd89089f975f9c32729945c941b92 Mon Sep 17 00:00:00 2001 From: Alexander J <741037+jaegeral@users.noreply.github.com> Date: Mon, 22 Jun 2026 15:30:11 +0200 Subject: [PATCH 02/11] Update cli_client/python/timesketch_cli_client/commands/sketch.py Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> --- cli_client/python/timesketch_cli_client/commands/sketch.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli_client/python/timesketch_cli_client/commands/sketch.py b/cli_client/python/timesketch_cli_client/commands/sketch.py index c0f3ad3d98..eae4df7f58 100644 --- a/cli_client/python/timesketch_cli_client/commands/sketch.py +++ b/cli_client/python/timesketch_cli_client/commands/sketch.py @@ -367,7 +367,7 @@ def delete_sketch(ctx: click.Context, force_delete: bool) -> None: # returns a 404, which raises a RuntimeError. timeline_desc = timeline.description timeline_status = timeline.status - except RuntimeError: + except RuntimeError as e: timeline_desc = "N/A" timeline_status = "N/A" click.echo( From 89fe23d48f379be2c8150bf643b8a9c7736b004f Mon Sep 17 00:00:00 2001 From: jaegeral Date: Mon, 22 Jun 2026 13:38:20 +0000 Subject: [PATCH 03/11] fix lint --- .../timesketch_cli_client/commands/sketch.py | 21 ++++++++++++++++--- end_to_end_tests/cli_client_e2e_test.py | 18 +++++++++------- 2 files changed, 28 insertions(+), 11 deletions(-) diff --git a/cli_client/python/timesketch_cli_client/commands/sketch.py b/cli_client/python/timesketch_cli_client/commands/sketch.py index eae4df7f58..34be6d027b 100644 --- a/cli_client/python/timesketch_cli_client/commands/sketch.py +++ b/cli_client/python/timesketch_cli_client/commands/sketch.py @@ -21,6 +21,7 @@ from timesketch_cli_client.commands import attribute as attribute_command from timesketch_api_client import search +from timesketch_api_client.error import NotFoundError @click.group("sketch") @@ -356,18 +357,32 @@ def delete_sketch(ctx: click.Context, force_delete: bool) -> None: # Dryrun: if not force_delete: click.echo("Would delete the following things (use --force_delete to execute)") + try: + sketch_desc = sketch.description + sketch_status = sketch.status + sketch_labels = sketch.labels + except (RuntimeError, NotFoundError) as e: # pylint: disable=unused-variable + sketch_desc = "N/A" + sketch_status = "N/A" + sketch_labels = "N/A" + click.echo( - f"Sketch: {sketch.id} {sketch.name} {sketch.description} {sketch.status} Labels: {sketch.labels}" # pylint: disable=line-too-long + f"Sketch: {sketch.id} {sketch.name} {sketch_desc} {sketch_status} Labels: {sketch_labels}" # pylint: disable=line-too-long ) - for timeline in sketch.list_timelines(): + try: + timelines = sketch.list_timelines() + except (RuntimeError, NotFoundError) as e: # pylint: disable=unused-variable + timelines = [] + + for timeline in timelines: try: # timeline.description and timeline.status lazy-load from the API. # If the sketch is already soft-deleted, the timeline endpoint # returns a 404, which raises a RuntimeError. timeline_desc = timeline.description timeline_status = timeline.status - except RuntimeError as e: + except (RuntimeError, NotFoundError) as e: # pylint: disable=unused-variable timeline_desc = "N/A" timeline_status = "N/A" click.echo( diff --git a/end_to_end_tests/cli_client_e2e_test.py b/end_to_end_tests/cli_client_e2e_test.py index 2847782a4f..94ca2c1379 100644 --- a/end_to_end_tests/cli_client_e2e_test.py +++ b/end_to_end_tests/cli_client_e2e_test.py @@ -220,34 +220,36 @@ def test_cli_sketch_delete_soft_deleted(self): # Create a new sketch to be soft-deleted and then hard-deleted sketch_name = f"cli_soft_delete_test_{uuid.uuid4().hex}" sketch = self.api.create_sketch(name=sketch_name) - + # We need a timeline to trigger the loop in sketch.py self.import_timeline("evtx_part.csv", sketch=sketch) - + # Soft delete the sketch sketch.delete(force_delete=False) - + # Now try to delete it via CLI (dry-run first, then force) cli_ctx_obj = E2ECliContextObject( api_client=self.api, sketch_instance=sketch, output_format="text", ) - + # Dry-run result = self.runner.invoke(sketch_group, ["delete"], obj=cli_ctx_obj) self.assertions.assertEqual( result.exit_code, 0, - f"CLI command 'sketch delete' (dry-run) failed on soft-deleted sketch.\nOutput:\n{result.output}\nException:\n{result.exception}" + f"CLI command 'sketch delete' (dry-run) failed on soft-deleted sketch.\nOutput:\n{result.output}\nException:\n{result.exception}", ) - + # Force-delete - result_force = self.runner.invoke(sketch_group, ["delete", "--force_delete"], obj=cli_ctx_obj) + result_force = self.runner.invoke( + sketch_group, ["delete", "--force_delete"], obj=cli_ctx_obj + ) self.assertions.assertEqual( result_force.exit_code, 0, - f"CLI command 'sketch delete --force_delete' failed on soft-deleted sketch.\nOutput:\n{result_force.output}\nException:\n{result_force.exception}" + f"CLI command 'sketch delete --force_delete' failed on soft-deleted sketch.\nOutput:\n{result_force.output}\nException:\n{result_force.exception}", ) From 3cff227c1b8e35dafdc7be4e301e8d46a8d41a37 Mon Sep 17 00:00:00 2001 From: jaegeral Date: Mon, 22 Jun 2026 13:49:50 +0000 Subject: [PATCH 04/11] fix lint --- .../python/timesketch_cli_client/commands/sketch.py | 10 ++++++---- end_to_end_tests/cli_client_e2e_test.py | 5 +++-- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/cli_client/python/timesketch_cli_client/commands/sketch.py b/cli_client/python/timesketch_cli_client/commands/sketch.py index 34be6d027b..f4387bc887 100644 --- a/cli_client/python/timesketch_cli_client/commands/sketch.py +++ b/cli_client/python/timesketch_cli_client/commands/sketch.py @@ -358,16 +358,18 @@ def delete_sketch(ctx: click.Context, force_delete: bool) -> None: if not force_delete: click.echo("Would delete the following things (use --force_delete to execute)") try: + sketch_name = sketch.name sketch_desc = sketch.description sketch_status = sketch.status sketch_labels = sketch.labels except (RuntimeError, NotFoundError) as e: # pylint: disable=unused-variable + sketch_name = "N/A" sketch_desc = "N/A" sketch_status = "N/A" sketch_labels = "N/A" click.echo( - f"Sketch: {sketch.id} {sketch.name} {sketch_desc} {sketch_status} Labels: {sketch_labels}" # pylint: disable=line-too-long + f"Sketch: {sketch.id} {sketch_name} {sketch_desc} {sketch_status} Labels: {sketch_labels}" # pylint: disable=line-too-long ) try: @@ -393,10 +395,10 @@ def delete_sketch(ctx: click.Context, force_delete: bool) -> None: # --- Check the response for success or error --- try: sketch.delete(force_delete=force_delete) - click.echo(f"Sketch {sketch.id} '{sketch.name}' successfully deleted.") - except RuntimeError as e: + click.echo(f"Sketch {sketch.id} '{sketch_name}' successfully deleted.") + except (RuntimeError, NotFoundError) as e: click.echo( - f"Failed to delete sketch {sketch.id} '{sketch.name}'. Error: {e}" + f"Failed to delete sketch {sketch.id} '{sketch_name}'. Error: {e}" ) ctx.exit(1) diff --git a/end_to_end_tests/cli_client_e2e_test.py b/end_to_end_tests/cli_client_e2e_test.py index 94ca2c1379..1d52fdf6a2 100644 --- a/end_to_end_tests/cli_client_e2e_test.py +++ b/end_to_end_tests/cli_client_e2e_test.py @@ -248,9 +248,10 @@ def test_cli_sketch_delete_soft_deleted(self): ) self.assertions.assertEqual( result_force.exit_code, - 0, - f"CLI command 'sketch delete --force_delete' failed on soft-deleted sketch.\nOutput:\n{result_force.output}\nException:\n{result_force.exception}", + 1, + f"CLI command 'sketch delete --force_delete' unexpectedly succeeded on a soft-deleted sketch for a non-admin.\nOutput:\n{result_force.output}\nException:\n{result_force.exception}", ) + self.assertions.assertIn("Failed to delete sketch", result_force.output) # Register the new test class with the test manager From 48d2e5dd31acf0d97e987801a74205fa42dd0469 Mon Sep 17 00:00:00 2001 From: jaegeral Date: Mon, 22 Jun 2026 13:54:41 +0000 Subject: [PATCH 05/11] pylint --- end_to_end_tests/cli_client_e2e_test.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/end_to_end_tests/cli_client_e2e_test.py b/end_to_end_tests/cli_client_e2e_test.py index 1d52fdf6a2..d8cd0c42d4 100644 --- a/end_to_end_tests/cli_client_e2e_test.py +++ b/end_to_end_tests/cli_client_e2e_test.py @@ -239,7 +239,7 @@ def test_cli_sketch_delete_soft_deleted(self): self.assertions.assertEqual( result.exit_code, 0, - f"CLI command 'sketch delete' (dry-run) failed on soft-deleted sketch.\nOutput:\n{result.output}\nException:\n{result.exception}", + f"CLI command 'sketch delete' (dry-run) failed on soft-deleted sketch.\nOutput:\n{result.output}\nException:\n{result.exception}", # pylint: disable=line-too-long ) # Force-delete @@ -249,7 +249,7 @@ def test_cli_sketch_delete_soft_deleted(self): self.assertions.assertEqual( result_force.exit_code, 1, - f"CLI command 'sketch delete --force_delete' unexpectedly succeeded on a soft-deleted sketch for a non-admin.\nOutput:\n{result_force.output}\nException:\n{result_force.exception}", + f"CLI command 'sketch delete --force_delete' unexpectedly succeeded on a soft-deleted sketch for a non-admin.\nOutput:\n{result_force.output}\nException:\n{result_force.exception}", # pylint: disable=line-too-long ) self.assertions.assertIn("Failed to delete sketch", result_force.output) From fd82b96eb4d32a3d389d370fbc9a8563d3709981 Mon Sep 17 00:00:00 2001 From: jaegeral Date: Mon, 22 Jun 2026 14:16:03 +0000 Subject: [PATCH 06/11] improve --- .../python/timesketch_api_client/sketch.py | 10 ++- .../timesketch_cli_client/commands/sketch.py | 64 ++++++++++++------- end_to_end_tests/cli_client_e2e_test.py | 9 ++- 3 files changed, 53 insertions(+), 30 deletions(-) diff --git a/api_client/python/timesketch_api_client/sketch.py b/api_client/python/timesketch_api_client/sketch.py index ad74426d2a..a93ceee1c9 100644 --- a/api_client/python/timesketch_api_client/sketch.py +++ b/api_client/python/timesketch_api_client/sketch.py @@ -502,14 +502,18 @@ def delete(self, force_delete=False): # Check the return status. If it's not a success (20x), # error_message will raise a RuntimeError. if not error.check_return_status(response, logger): + if response.status_code == definitions.HTTP_STATUS_CODE_NOT_FOUND: + error.error_message( + response, + message=f"Failed to delete sketch {self.id}", + error=error.NotFoundError, + ) error.error_message( response, message=f"Failed to delete sketch {self.id}", - error=RuntimeError, ) else: - return error.check_return_status(response, logger) - return True + return True def add_to_acl( self, diff --git a/cli_client/python/timesketch_cli_client/commands/sketch.py b/cli_client/python/timesketch_cli_client/commands/sketch.py index f4387bc887..af1be2df64 100644 --- a/cli_client/python/timesketch_cli_client/commands/sketch.py +++ b/cli_client/python/timesketch_cli_client/commands/sketch.py @@ -349,46 +349,57 @@ def delete_sketch(ctx: click.Context, force_delete: bool) -> None: force_delete (bool): If true, delete immediately. """ sketch = ctx.obj.sketch - # if sketch is archived, exit - if sketch.is_archived(): - click.echo("Error Sketch is archived") - ctx.exit(1) - # Dryrun: - if not force_delete: - click.echo("Would delete the following things (use --force_delete to execute)") try: - sketch_name = sketch.name - sketch_desc = sketch.description - sketch_status = sketch.status - sketch_labels = sketch.labels - except (RuntimeError, NotFoundError) as e: # pylint: disable=unused-variable - sketch_name = "N/A" + is_archived = sketch.is_archived() + except NotFoundError: + click.echo(f"Warning: Sketch {sketch.id} appears to be soft-deleted or inaccessible.") + if not force_delete: + click.echo("If you want to permanently delete it, use --force_delete") + ctx.exit(1) + # Proceed with force delete, but we don't know the sketch name. + is_archived = False + sketch_name = "" sketch_desc = "N/A" sketch_status = "N/A" sketch_labels = "N/A" + timelines = [] + else: + if is_archived: + click.echo("Error Sketch is archived") + ctx.exit(1) + + try: + sketch_name = sketch.name + sketch_desc = sketch.description + sketch_status = sketch.status + sketch_labels = sketch.labels + timelines = sketch.list_timelines() + except NotFoundError: + sketch_name = "" + sketch_desc = "N/A" + sketch_status = "N/A" + sketch_labels = "N/A" + timelines = [] + + # Dryrun: + if not force_delete: + click.echo("Would delete the following things (use --force_delete to execute)") click.echo( - f"Sketch: {sketch.id} {sketch_name} {sketch_desc} {sketch_status} Labels: {sketch_labels}" # pylint: disable=line-too-long + f"Sketch: {sketch.id} {sketch_name} {sketch_desc} {sketch_status} Labels: {sketch_labels}" ) - try: - timelines = sketch.list_timelines() - except (RuntimeError, NotFoundError) as e: # pylint: disable=unused-variable - timelines = [] - for timeline in timelines: try: # timeline.description and timeline.status lazy-load from the API. - # If the sketch is already soft-deleted, the timeline endpoint - # returns a 404, which raises a RuntimeError. timeline_desc = timeline.description timeline_status = timeline.status - except (RuntimeError, NotFoundError) as e: # pylint: disable=unused-variable + except NotFoundError: timeline_desc = "N/A" timeline_status = "N/A" click.echo( - f" Timeline: {timeline.id} {timeline.name} {timeline_desc} {timeline_status}" # pylint: disable=line-too-long + f" Timeline: {timeline.id} {timeline.name} {timeline_desc} {timeline_status}" ) if force_delete: @@ -396,7 +407,12 @@ def delete_sketch(ctx: click.Context, force_delete: bool) -> None: try: sketch.delete(force_delete=force_delete) click.echo(f"Sketch {sketch.id} '{sketch_name}' successfully deleted.") - except (RuntimeError, NotFoundError) as e: + except NotFoundError: + click.echo( + f"Failed to delete sketch {sketch.id} '{sketch_name}'. Error: Sketch was not found (perhaps already permanently deleted?)." + ) + ctx.exit(1) + except RuntimeError as e: click.echo( f"Failed to delete sketch {sketch.id} '{sketch_name}'. Error: {e}" ) diff --git a/end_to_end_tests/cli_client_e2e_test.py b/end_to_end_tests/cli_client_e2e_test.py index d8cd0c42d4..ca145cfaf8 100644 --- a/end_to_end_tests/cli_client_e2e_test.py +++ b/end_to_end_tests/cli_client_e2e_test.py @@ -227,10 +227,13 @@ def test_cli_sketch_delete_soft_deleted(self): # Soft delete the sketch sketch.delete(force_delete=False) + # Get a fresh instance of the sketch so cached values are cleared + fresh_sketch = self.api.get_sketch(sketch.id) + # Now try to delete it via CLI (dry-run first, then force) cli_ctx_obj = E2ECliContextObject( api_client=self.api, - sketch_instance=sketch, + sketch_instance=fresh_sketch, output_format="text", ) @@ -238,8 +241,8 @@ def test_cli_sketch_delete_soft_deleted(self): result = self.runner.invoke(sketch_group, ["delete"], obj=cli_ctx_obj) self.assertions.assertEqual( result.exit_code, - 0, - f"CLI command 'sketch delete' (dry-run) failed on soft-deleted sketch.\nOutput:\n{result.output}\nException:\n{result.exception}", # pylint: disable=line-too-long + 1, + f"CLI command 'sketch delete' (dry-run) failed to exit with 1 on soft-deleted sketch.\nOutput:\n{result.output}\nException:\n{result.exception}", # pylint: disable=line-too-long ) # Force-delete From dd2da188df839762ffcde59479988795667a8a9b Mon Sep 17 00:00:00 2001 From: jaegeral Date: Mon, 22 Jun 2026 14:24:02 +0000 Subject: [PATCH 07/11] improve --- .../python/timesketch_api_client/sketch.py | 4 +- .../timesketch_cli_client/commands/sketch.py | 60 +++++++++---------- 2 files changed, 32 insertions(+), 32 deletions(-) diff --git a/api_client/python/timesketch_api_client/sketch.py b/api_client/python/timesketch_api_client/sketch.py index a93ceee1c9..7615ec7434 100644 --- a/api_client/python/timesketch_api_client/sketch.py +++ b/api_client/python/timesketch_api_client/sketch.py @@ -512,8 +512,8 @@ def delete(self, force_delete=False): response, message=f"Failed to delete sketch {self.id}", ) - else: - return True + + return True def add_to_acl( self, diff --git a/cli_client/python/timesketch_cli_client/commands/sketch.py b/cli_client/python/timesketch_cli_client/commands/sketch.py index af1be2df64..a174b04817 100644 --- a/cli_client/python/timesketch_cli_client/commands/sketch.py +++ b/cli_client/python/timesketch_cli_client/commands/sketch.py @@ -350,56 +350,56 @@ def delete_sketch(ctx: click.Context, force_delete: bool) -> None: """ sketch = ctx.obj.sketch + # Initialize with default values. Preserve cached sketch_name if it exists. + sketch_name = getattr(sketch, "_sketch_name", None) or "" + sketch_desc = "N/A" + sketch_status = "N/A" + sketch_labels = "N/A" + timelines = [] + try: is_archived = sketch.is_archived() - except NotFoundError: - click.echo(f"Warning: Sketch {sketch.id} appears to be soft-deleted or inaccessible.") + except NotFoundError as e: # pylint: disable=unused-variable + click.echo( + f"Warning: Sketch {sketch.id} appears to be soft-deleted or inaccessible." + ) if not force_delete: click.echo("If you want to permanently delete it, use --force_delete") ctx.exit(1) - # Proceed with force delete, but we don't know the sketch name. is_archived = False - sketch_name = "" - sketch_desc = "N/A" - sketch_status = "N/A" - sketch_labels = "N/A" - timelines = [] - else: - if is_archived: - click.echo("Error Sketch is archived") - ctx.exit(1) - try: - sketch_name = sketch.name - sketch_desc = sketch.description - sketch_status = sketch.status - sketch_labels = sketch.labels - timelines = sketch.list_timelines() - except NotFoundError: - sketch_name = "" - sketch_desc = "N/A" - sketch_status = "N/A" - sketch_labels = "N/A" - timelines = [] + if is_archived: + click.echo("Error Sketch is archived") + ctx.exit(1) + + try: + sketch_name = sketch.name + sketch_desc = sketch.description + sketch_status = sketch.status + sketch_labels = sketch.labels + timelines = sketch.list_timelines() + except NotFoundError as e: # pylint: disable=unused-variable + pass # Dryrun: if not force_delete: click.echo("Would delete the following things (use --force_delete to execute)") click.echo( - f"Sketch: {sketch.id} {sketch_name} {sketch_desc} {sketch_status} Labels: {sketch_labels}" + f"Sketch: {sketch.id} {sketch_name} {sketch_desc} {sketch_status} Labels: {sketch_labels}" # pylint: disable=line-too-long ) for timeline in timelines: + timeline_desc = "N/A" + timeline_status = "N/A" try: # timeline.description and timeline.status lazy-load from the API. timeline_desc = timeline.description timeline_status = timeline.status - except NotFoundError: - timeline_desc = "N/A" - timeline_status = "N/A" + except NotFoundError as e: # pylint: disable=unused-variable + pass click.echo( - f" Timeline: {timeline.id} {timeline.name} {timeline_desc} {timeline_status}" + f" Timeline: {timeline.id} {timeline.name} {timeline_desc} {timeline_status}" # pylint: disable=line-too-long ) if force_delete: @@ -409,7 +409,7 @@ def delete_sketch(ctx: click.Context, force_delete: bool) -> None: click.echo(f"Sketch {sketch.id} '{sketch_name}' successfully deleted.") except NotFoundError: click.echo( - f"Failed to delete sketch {sketch.id} '{sketch_name}'. Error: Sketch was not found (perhaps already permanently deleted?)." + f"Failed to delete sketch {sketch.id} '{sketch_name}'. Error: Sketch was not found (perhaps already permanently deleted?)." # pylint: disable=line-too-long ) ctx.exit(1) except RuntimeError as e: From ca5bd25e640e0c9165c6830835e02b12c7093d35 Mon Sep 17 00:00:00 2001 From: jaegeral Date: Mon, 17 Aug 2026 09:37:43 +0000 Subject: [PATCH 08/11] have a better error message when not using force_delete and return redacted infos for admins calling list_timelines() --- .../python/timesketch_api_client/sketch.py | 10 ++-- .../timesketch_cli_client/commands/sketch.py | 11 ++--- end_to_end_tests/cli_client_e2e_test.py | 4 +- timesketch/api/v1/resources/sketch.py | 49 ++++++++++++++++++- timesketch/api/v1/resources_test.py | 41 ++++++++++++++++ 5 files changed, 103 insertions(+), 12 deletions(-) diff --git a/api_client/python/timesketch_api_client/sketch.py b/api_client/python/timesketch_api_client/sketch.py index 7615ec7434..815a2864d5 100644 --- a/api_client/python/timesketch_api_client/sketch.py +++ b/api_client/python/timesketch_api_client/sketch.py @@ -995,12 +995,16 @@ def list_timelines(self): return timelines for timeline_dict in objects[0].get("timelines", []): + searchindex = timeline_dict.get("searchindex") + searchindex_name = "" + if isinstance(searchindex, dict): + searchindex_name = searchindex.get("index_name", "") timeline_obj = timeline.Timeline( - timeline_id=timeline_dict["id"], + timeline_id=timeline_dict.get("id"), sketch_id=self.id, api=self.api, - name=timeline_dict["name"], - searchindex=timeline_dict["searchindex"]["index_name"], + name=timeline_dict.get("name"), + searchindex=searchindex_name, ) timelines.append(timeline_obj) return timelines diff --git a/cli_client/python/timesketch_cli_client/commands/sketch.py b/cli_client/python/timesketch_cli_client/commands/sketch.py index c5a3a85b42..32111f5b34 100644 --- a/cli_client/python/timesketch_cli_client/commands/sketch.py +++ b/cli_client/python/timesketch_cli_client/commands/sketch.py @@ -337,17 +337,14 @@ def delete_sketch(ctx: click.Context, force_delete: bool) -> None: try: is_archived = sketch.is_archived() - except NotFoundError as e: # pylint: disable=unused-variable + except NotFoundError: click.echo( - f"Warning: Sketch {sketch.id} appears to be soft-deleted or inaccessible." + f"Error: Sketch {sketch.id} not found or you do not have permission to access it." ) - if not force_delete: - click.echo("If you want to permanently delete it, use --force_delete") - ctx.exit(1) - is_archived = False + ctx.exit(1) if is_archived: - click.echo("Error Sketch is archived") + click.echo("Error: Sketch is archived.") ctx.exit(1) try: diff --git a/end_to_end_tests/cli_client_e2e_test.py b/end_to_end_tests/cli_client_e2e_test.py index ca145cfaf8..dac45b0471 100644 --- a/end_to_end_tests/cli_client_e2e_test.py +++ b/end_to_end_tests/cli_client_e2e_test.py @@ -254,7 +254,9 @@ def test_cli_sketch_delete_soft_deleted(self): 1, f"CLI command 'sketch delete --force_delete' unexpectedly succeeded on a soft-deleted sketch for a non-admin.\nOutput:\n{result_force.output}\nException:\n{result_force.exception}", # pylint: disable=line-too-long ) - self.assertions.assertIn("Failed to delete sketch", result_force.output) + self.assertions.assertIn( + "not found or you do not have permission", result_force.output + ) # Register the new test class with the test manager diff --git a/timesketch/api/v1/resources/sketch.py b/timesketch/api/v1/resources/sketch.py index 388d99d01a..5bf99ed259 100644 --- a/timesketch/api/v1/resources/sketch.py +++ b/timesketch/api/v1/resources/sketch.py @@ -328,6 +328,12 @@ def _get_sketch_for_admin(sketch: Sketch): A limited view of a sketch in JSON (instance of flask.wrappers.Response) """ + if not current_user.admin: + abort( + HTTP_STATUS_CODE_FORBIDDEN, + "Admin privileges required to access this view.", + ) + if sketch.get_status.status == "archived": status = "archived" elif sketch.get_status.status == "deleted": @@ -335,12 +341,53 @@ def _get_sketch_for_admin(sketch: Sketch): else: status = "admin_view" + has_read_permission = sketch.has_permission(current_user, "read") + timelines = [] + for timeline in sketch.timelines: + timeline_status = ( + timeline.get_status.status if timeline.get_status else "unknown" + ) + if has_read_permission: + timelines.append( + { + "id": timeline.id, + "name": timeline.name, + "description": timeline.description, + "color": timeline.color, + "status": [{"id": 0, "status": timeline_status}], + "searchindex": { + "index_name": ( + timeline.searchindex.index_name + if timeline.searchindex + else "" + ) + }, + "created_at": timeline.created_at, + "updated_at": timeline.updated_at, + "deleted": timeline_status == "deleted", + } + ) + else: # for admins, only redacted information are needed + timelines.append( + { + "id": timeline.id, + "name": "", + "description": "", + "color": "", + "status": [{"id": 0, "status": timeline_status}], + "searchindex": {"index_name": ""}, + "created_at": timeline.created_at, + "updated_at": timeline.updated_at, + "deleted": timeline_status == "deleted", + } + ) + sketch_fields = { "id": sketch.id, "name": sketch.name, "description": sketch.description, "user": {"username": current_user.username}, - "timelines": [], + "timelines": timelines, "stories": [], "active_timelines": [], "label_string": sketch.label_string, diff --git a/timesketch/api/v1/resources_test.py b/timesketch/api/v1/resources_test.py index 9305789f37..03046be161 100644 --- a/timesketch/api/v1/resources_test.py +++ b/timesketch/api/v1/resources_test.py @@ -271,6 +271,47 @@ def test_sketch_acl(self): response = self.client.get("/api/v1/sketches/2/") self.assert403(response) + def test_get_sketch_for_admin_no_read_permission(self): + """Admin request to get a sketch without explicit read permission.""" + self.login_admin() + response = self.client.get("/api/v1/sketches/1/") + self.assert200(response) + sketch_obj = response.json["objects"][0] + self.assertEqual(sketch_obj["id"], 1) + self.assertEqual(sketch_obj["status"][0]["status"], "admin_view") + self.assertEqual(len(sketch_obj["timelines"]), 1) + self.assertEqual(sketch_obj["timelines"][0]["name"], "") + self.assertEqual(sketch_obj["timelines"][0]["description"], "") + + @mock.patch("timesketch.api.v1.resources.OpenSearchDataStore", MockDataStore) + def test_get_sketch_for_admin_soft_deleted(self): + """Admin request to get a soft-deleted sketch with read permission.""" + # Grant admin explicit read permission on sketch 1 + self.sketch1.grant_permission(permission="read", user=self.useradmin) + self.login_admin() + + # Soft-delete sketch 1 + delete_response = self.client.delete(self.resource_url) + self.assert200(delete_response) + + # Admin gets the soft-deleted sketch + response = self.client.get(self.resource_url) + self.assert200(response) + sketch_obj = response.json["objects"][0] + self.assertEqual(sketch_obj["status"][0]["status"], "deleted") + self.assertEqual(len(sketch_obj["timelines"]), 1) + self.assertEqual(sketch_obj["timelines"][0]["name"], "Timeline 1") + + def test_get_sketch_for_admin_non_admin_forbidden(self): + """Non-admin invocation of _get_sketch_for_admin is rejected.""" + self.login() + with self.client: + self.client.get("/") + from timesketch.api.v1.resources.sketch import SketchResource + from werkzeug.exceptions import Forbidden + with self.assertRaises(Forbidden): + SketchResource._get_sketch_for_admin(self.sketch1) + def test_create_a_sketch(self): """Authenticated request to create a sketch.""" self.login() From bbc3bc277bd3131bdb098eecdeffbd22241b614b Mon Sep 17 00:00:00 2001 From: jaegeral Date: Mon, 17 Aug 2026 10:29:55 +0000 Subject: [PATCH 09/11] lint and black --- api_client/python/timesketch_api_client/sketch.py | 2 +- cli_client/python/timesketch_cli_client/commands/sketch.py | 3 ++- timesketch/api/v1/resources/sketch.py | 2 +- timesketch/api/v1/resources_test.py | 7 ++++--- 4 files changed, 8 insertions(+), 6 deletions(-) diff --git a/api_client/python/timesketch_api_client/sketch.py b/api_client/python/timesketch_api_client/sketch.py index 815a2864d5..97776ecf24 100644 --- a/api_client/python/timesketch_api_client/sketch.py +++ b/api_client/python/timesketch_api_client/sketch.py @@ -998,7 +998,7 @@ def list_timelines(self): searchindex = timeline_dict.get("searchindex") searchindex_name = "" if isinstance(searchindex, dict): - searchindex_name = searchindex.get("index_name", "") + searchindex_name = searchindex.get("index_name") or "" timeline_obj = timeline.Timeline( timeline_id=timeline_dict.get("id"), sketch_id=self.id, diff --git a/cli_client/python/timesketch_cli_client/commands/sketch.py b/cli_client/python/timesketch_cli_client/commands/sketch.py index 32111f5b34..b57884e259 100644 --- a/cli_client/python/timesketch_cli_client/commands/sketch.py +++ b/cli_client/python/timesketch_cli_client/commands/sketch.py @@ -339,7 +339,8 @@ def delete_sketch(ctx: click.Context, force_delete: bool) -> None: is_archived = sketch.is_archived() except NotFoundError: click.echo( - f"Error: Sketch {sketch.id} not found or you do not have permission to access it." + f"Error: Sketch {sketch.id} not found or you do not have permission " + f"to access it." ) ctx.exit(1) diff --git a/timesketch/api/v1/resources/sketch.py b/timesketch/api/v1/resources/sketch.py index 5bf99ed259..85f0d799d0 100644 --- a/timesketch/api/v1/resources/sketch.py +++ b/timesketch/api/v1/resources/sketch.py @@ -367,7 +367,7 @@ def _get_sketch_for_admin(sketch: Sketch): "deleted": timeline_status == "deleted", } ) - else: # for admins, only redacted information are needed + else: # for admins, only redacted information are needed timelines.append( { "id": timeline.id, diff --git a/timesketch/api/v1/resources_test.py b/timesketch/api/v1/resources_test.py index 03046be161..3e8011f1f6 100644 --- a/timesketch/api/v1/resources_test.py +++ b/timesketch/api/v1/resources_test.py @@ -18,6 +18,7 @@ import tempfile import json from unittest import mock +from werkzeug.exceptions import Forbidden from timesketch.lib.definitions import HTTP_STATUS_CODE_BAD_REQUEST from timesketch.lib.definitions import HTTP_STATUS_CODE_CREATED @@ -46,6 +47,7 @@ from timesketch.lib.llms.providers import manager as llm_manager from timesketch.lib.llms.features import interface as feature_interface from timesketch.api.v1.resources import llm +from timesketch.api.v1.resources import sketch as sketch_resources class ResourceMixinTest(BaseTest): @@ -307,10 +309,9 @@ def test_get_sketch_for_admin_non_admin_forbidden(self): self.login() with self.client: self.client.get("/") - from timesketch.api.v1.resources.sketch import SketchResource - from werkzeug.exceptions import Forbidden + # pylint: disable=protected-access with self.assertRaises(Forbidden): - SketchResource._get_sketch_for_admin(self.sketch1) + sketch_resources.SketchResource._get_sketch_for_admin(self.sketch1) def test_create_a_sketch(self): """Authenticated request to create a sketch.""" From b7fbe94f17c1441f4d7004f05c3ca7ccc1d784a8 Mon Sep 17 00:00:00 2001 From: Alexander J <741037+jaegeral@users.noreply.github.com> Date: Thu, 24 Sep 2026 12:07:40 +0200 Subject: [PATCH 10/11] Update cli_client/python/timesketch_cli_client/commands/sketch.py Co-authored-by: Janosch <99879757+jkppr@users.noreply.github.com> --- cli_client/python/timesketch_cli_client/commands/sketch.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli_client/python/timesketch_cli_client/commands/sketch.py b/cli_client/python/timesketch_cli_client/commands/sketch.py index 5c27d90350..01e251ba58 100644 --- a/cli_client/python/timesketch_cli_client/commands/sketch.py +++ b/cli_client/python/timesketch_cli_client/commands/sketch.py @@ -356,7 +356,7 @@ def delete_sketch(ctx: click.Context, force_delete: bool) -> None: sketch_status = sketch.status sketch_labels = sketch.labels timelines = sketch.list_timelines() - except NotFoundError as e: # pylint: disable=unused-variable + except NotFoundError: pass # Dryrun: From 3bc044a06640d6e871f949c85edc9652860a0e6a Mon Sep 17 00:00:00 2001 From: Alexander J <741037+jaegeral@users.noreply.github.com> Date: Thu, 24 Sep 2026 12:07:48 +0200 Subject: [PATCH 11/11] Update cli_client/python/timesketch_cli_client/commands/sketch.py Co-authored-by: Janosch <99879757+jkppr@users.noreply.github.com> --- cli_client/python/timesketch_cli_client/commands/sketch.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli_client/python/timesketch_cli_client/commands/sketch.py b/cli_client/python/timesketch_cli_client/commands/sketch.py index 01e251ba58..97e6e08f24 100644 --- a/cli_client/python/timesketch_cli_client/commands/sketch.py +++ b/cli_client/python/timesketch_cli_client/commands/sketch.py @@ -374,7 +374,7 @@ def delete_sketch(ctx: click.Context, force_delete: bool) -> None: # timeline.description and timeline.status lazy-load from the API. timeline_desc = timeline.description timeline_status = timeline.status - except NotFoundError as e: # pylint: disable=unused-variable + except NotFoundError: pass click.echo( f" Timeline: {timeline.id} {timeline.name} {timeline_desc} {timeline_status}" # pylint: disable=line-too-long