-
Notifications
You must be signed in to change notification settings - Fork 1.7k
docs: update RFC-001 progress and split Phase 8 #528
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
Closed
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -1,9 +1,9 @@ | ||||||
| # RFC-001: Architecture Refactoring | ||||||
|
|
||||||
| **Author**: moonD4rk | ||||||
| **Status**: Proposed | ||||||
| **Status**: In Progress (Phase 8a) | ||||||
| **Created**: 2025-09-01 | ||||||
| **Updated**: 2026-03-22 | ||||||
| **Updated**: 2026-03-30 | ||||||
|
|
||||||
| ## Abstract | ||||||
|
|
||||||
|
|
@@ -747,18 +747,30 @@ data.Output(dir, b.Name(), format) // output whatever succeeded | |||||
|
|
||||||
| ## 7. Implementation Order | ||||||
|
|
||||||
| | Phase | Scope | Risk | | ||||||
| |-------|-------|------| | ||||||
| | 1 | `types/category.go` + `types/models.go` + `browserdata/browserdata.go` | Zero — new files only | | ||||||
| | 2 | `utils/sqliteutil/sqlite.go` + `query.go` | Zero — new files only | | ||||||
| | 3 | `crypto/version.go`, rename `AESCBCDecrypt` | Low — internal crypto changes | | ||||||
| | 4 | `crypto/keyretriever/` | Low — new package | | ||||||
| | 5 | `browser/chromium/source.go` + `extract_*.go` | Medium — new extract methods | | ||||||
| | 6 | `browser/firefox/source.go` + `extract_*.go` | Medium — new extract methods | | ||||||
| | 7 | `filemanager/session.go` | Low — new package | | ||||||
| | 8 | Wire `Extract()` + `Config` + `PickBrowsers()` | High — connects everything | | ||||||
| | 9 | Delete old code: `extractor/`, `browserdata/*/`, `imports.go` | High — removal | | ||||||
| | 10 | Update CLI, tests, cross-platform build verification | Medium | | ||||||
| | Phase | Scope | Risk | Status | | ||||||
| |-------|-------|------|--------| | ||||||
| | 1 | `types/category.go` + `types/models.go` + `browserdata/browserdata.go` | Zero — new files only | ✅ Done | | ||||||
| | 2 | `utils/sqliteutil/sqlite.go` + `query.go` | Zero — new files only | ✅ Done | | ||||||
| | 3 | `crypto/version.go`, rename `AESCBCDecrypt` | Low — internal crypto changes | ✅ Done | | ||||||
| | 4 | `crypto/keyretriever/` | Low — new package | ✅ Done | | ||||||
| | 5 | `browser/chromium/source.go` + `extract_*.go` | Medium — new extract methods | ✅ Done (#521) | | ||||||
| | 6 | `browser/firefox/source.go` + `extract_*.go` | Medium — new extract methods | ✅ Done (#527) | | ||||||
| | 7 | `filemanager/session.go` | Low — new package | ✅ Done (#516) | | ||||||
| | 8a | `browser/browser.go`: Browser interface + Category→extract routing | Medium | ⏳ | | ||||||
| | 8b | `browser/chromium/chromium.go`: Chromium Browser impl + keyretriever wiring | Medium | ⏳ | | ||||||
| | 8c | `browser/firefox/firefox.go` refactor: Firefox Browser impl + GetMasterKey migration | Medium | ⏳ | | ||||||
| | 8d | `browser/browser_*.go`: PickBrowsers() + platform browser discovery | High | ⏳ | | ||||||
| | 9 | Delete old code: `extractor/`, `browserdata/*/`, `imports.go` | High — removal | ⏳ | | ||||||
| | 10 | Update CLI, tests, cross-platform build verification | Medium | ⏳ | | ||||||
|
|
||||||
| Phase 8 was split into 4 sub-phases because it connects all components: | ||||||
| - **8a** defines the interface contract (no system interaction) | ||||||
| - **8b** and **8c** implement the two browser engines (can be done in parallel) | ||||||
| - **8d** adds platform-specific browser discovery (depends on 8b+8c) | ||||||
|
|
||||||
| Additional PRs merged during implementation: | ||||||
| - #524: Cookie SHA256 hash stripping for Chrome 130+ | ||||||
| - #525: Issue tracking LocalStorage LevelDB parsing bugs | ||||||
|
|
||||||
| --- | ||||||
|
|
||||||
|
|
@@ -785,8 +797,13 @@ data.Output(dir, b.Name(), format) // output whatever succeeded | |||||
| ## 9. Open Questions | ||||||
|
|
||||||
| 1. **App-Bound Encryption (Chrome 127+ v20)**: `crypto/version.go` has the extension point. Implementation deferred until tested. | ||||||
| 2. **Firefox version detection**: is the key-length heuristic in `processMasterKey()` sufficient, or formalize it? | ||||||
| 3. **Sort direction**: standardize all categories to DESC by date? (Firefox history/download currently ASC) | ||||||
| 2. ~~**Firefox version detection**: is the key-length heuristic in `processMasterKey()` sufficient?~~ — **Resolved**: IV length (8=3DES, 16=AES-256-CBC) determines algorithm. Full key returned without truncation. See #498/#499. | ||||||
| 3. ~~**Sort direction**: standardize all categories to DESC by date?~~ — **Resolved**: preserved original sort behavior per browser engine (Firefox history ASC, Chromium history DESC). | ||||||
|
|
||||||
| ### Known Issues (tracked) | ||||||
|
|
||||||
| - **#522**: TimeEpoch/TimeStamp conditional heuristic removed — needs investigation | ||||||
|
||||||
| - **#522**: TimeEpoch/TimeStamp conditional heuristic removed — needs investigation | |
| - **#522**: TimeEpoch/TimeStamp conditional heuristic and `time.Local`/magic-number handling in `utils/typeutil/typeutil.go` to be refactored — pending |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The section title says “Additional PRs merged during implementation”, but
#525is an issue (LevelDB parsing bug) rather than a PR. Consider either renaming this subsection to include issues too, or moving#525into a separate “Additional issues” list to avoid mislabeling.