Skip to content

fix reconciled-orphan health export leak + stale telemetry docs - #313

Closed
abdulsaheel wants to merge 1 commit into
audit/fixes-round3-edgefrom
audit/fixes-round4-edge
Closed

fix reconciled-orphan health export leak + stale telemetry docs#313
abdulsaheel wants to merge 1 commit into
audit/fixes-round3-edgefrom
audit/fixes-round4-edge

Conversation

@abdulsaheel

@abdulsaheel abdulsaheel commented Aug 29, 2026

Copy link
Copy Markdown
Collaborator

User description

round 3 only skipped the direct exportWorkoutId call for a crash-orphaned live workout, but the periodic exportAll pass still picked the row up a few minutes later since nothing on it said the end_ts was fake. added an end_ts_fabricated flag set at reconcile time and checked in _writeOneWorkout so it's skipped everywhere, not just that one call site.

also fixed README.md and SECURITY.md saying diagnostics telemetry is "on by default in github release builds" - it isn't, it's off by default everywhere until you flip it in settings, code and PRIVACY.md both already agreed on that.

Summary by Sourcery

Prevent fabricated completion times from leaking into health exports and align telemetry documentation with the actual opt-in behavior.

Bug Fixes:

  • Prevent reconciled orphaned workouts with fabricated end times from being exported to Apple Health or Health Connect through any export path.
  • Correct diagnostics telemetry documentation to state that collection is disabled by default in every build.

Documentation:

  • Update README.md and SECURITY.md to accurately describe diagnostics telemetry defaults.

Tests:

  • Add regression coverage for marking reconciled orphan sessions and preventing their health export.

PR Type

Bug fix, Documentation, Tests


Description

  • Prevents crash-orphaned workouts with fabricated end times from leaking into Apple Health or Health Connect during periodic exports.

  • Adds a database migration appending the end_ts_fabricated column to the sessions table to persistently flag reconciled orphans.

  • Updates README.md and SECURITY.md to accurately reflect that diagnostics telemetry is disabled by default across all builds.


Diagram Walkthrough

flowchart LR
  A["Stale Live Workout"] -- "Reconciled" --> B["AppState"]
  B -- "Sets end_ts_fabricated=1" --> C["LocalDb (sessions)"]
  C -- "Periodic Export" --> D["HealthExporter"]
  D -- "Skips export" --> E["Apple Health / Health Connect"]
Loading

File Walkthrough

Relevant files
Database schema
1 files
db.dart
Adds end_ts_fabricated column to sessions table                   
+12/-0   
Bug fix
2 files
health_export.dart
Skips health export for workouts with fabricated end times
+6/-0     
app_state.dart
Flags reconciled stale live workouts with end_ts_fabricated
+6/-0     
Tests
2 files
app_state_regressions_test.dart
Adds assertion for end_ts_fabricated flag on stale orphans
+4/-0     
health_workout_export_delete_gate_test.dart
Adds test verifying fabricated workouts are not exported 
+16/-0   
Documentation
2 files
README.md
Corrects telemetry default state documentation                     
+3/-3     
SECURITY.md
Corrects telemetry default state documentation                     
+2/-2     

the round-3 fix only skipped the direct export call for a crash-orphaned
workout, but the periodic exportAll pass still picked it up minutes later
since nothing marked the row as having a fake end_ts. flag it
(end_ts_fabricated) and skip it in _writeOneWorkout too.

also fixed README/SECURITY claiming diagnostics telemetry is on by default
on github builds - it's opt-in/off everywhere, matches PRIVACY.md and the
actual code.
@coderabbitai

coderabbitai Bot commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 8c616afd-9e83-4c38-af51-cd7b0baa407d

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@sourcery-ai

sourcery-ai Bot commented Aug 29, 2026

Copy link
Copy Markdown

Reviewer's Guide

The PR closes a periodic-export leak for crash-orphaned workouts by persisting and enforcing a fabricated end-timestamp marker, adds regression tests for the full behavior, and updates privacy documentation to state that diagnostics telemetry is disabled by default in every build.

Sequence diagram for preventing fabricated workout export

sequenceDiagram
    participant AppState
    participant LocalDb
    participant HealthExporter
    participant Health

    AppState->>LocalDb: putSession(status=done, end_ts, end_ts_fabricated=1)
    HealthExporter->>HealthExporter: exportAll()
    HealthExporter->>HealthExporter: _writeOneWorkout()
    alt end_ts_fabricated == 1
        HealthExporter-->>HealthExporter: skip
    else real completed workout
        HealthExporter->>Health: write workout
    end
Loading

File-Level Changes

Change Details Files
Prevent reconciled orphaned workouts from being exported as real health data on any export path.
  • Add a persisted end_ts_fabricated session flag with a default of 0.
  • Mark stale live sessions finalized without a real end timestamp as fabricated during reconciliation.
  • Skip fabricated-end sessions in the common workout write path.
  • Add regression coverage for reconciliation persistence and health-export suppression.
lib/data/db.dart
lib/state/app_state.dart
lib/health/health_export.dart
test/app_state_regressions_test.dart
test/health_workout_export_delete_gate_test.dart
Correct diagnostics telemetry documentation to match the implementation’s default behavior.
  • Document anonymous diagnostics as off by default in all builds and enabled only through profile settings.
  • Remove the outdated GitHub-release versus store-build distinction.
README.md
SECURITY.md

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've reviewed your changes and they look great!

Sourcery assessment

Needs a human reviewer. If the flag is set incorrectly, a legitimate workout could be skipped from Health export, but the session remains locally stored and can be repaired or re-exported after correcting the flag. Reverting restores the prior export behavior, while the existing fabricated-workout leak is not harm introduced by this change.


Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@github-actions

Copy link
Copy Markdown
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ No major issues detected

@github-actions

Copy link
Copy Markdown
Contributor

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Change expectation to isNull for skipped workouts

The _writeOneWorkout method explicitly returns null when it skips a workout (such as
a live or fabricated session). If exportWorkout propagates this return value, the
test should expect isNull rather than isFalse, as isFalse strictly checks for a
boolean false and will fail on null.

test/health_workout_export_delete_gate_test.dart [135-140]

 final ok = await HealthExporter().exportWorkout({
   ..._session(),
   'end_ts_fabricated': 1,
 });
 
-expect(ok, isFalse);
+expect(ok, isNull);
Suggestion importance[1-10]: 6

__

Why: The suggestion correctly identifies that _writeOneWorkout returns null for skipped workouts. If exportWorkout propagates this null value, the test will fail with isFalse, making this a valuable edge case to verify.

Low

@abdulsaheel

Copy link
Copy Markdown
Collaborator Author

squashed into #314 for one clean review — closing this round.

@abdulsaheel
abdulsaheel deleted the audit/fixes-round4-edge branch August 29, 2026 13:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant