Codex/security coderabbit fixes - #91
Merged
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (9)
📒 Files selected for processing (15)
💤 Files with no reviewable changes (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis PR updates DeepSec matcher parsing and lock recovery, refactors Compose headers, calibration controls, and screenshot previews, adjusts localized strings and tests, and adds Compose UI testing dependencies while upgrading Compose Stability metadata. ChangesDeepSec analysis and recovery tooling
Compose UI and preview refactoring
Compose testing and Stability dependency metadata
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fa35edb1ae
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+45
to
+46
| const nextShareStart = starts[index + 1] ?? content.length; | ||
| const end = findShareScopeEnd(content, start, nextShareStart); |
There was a problem hiding this comment.
Jos samassa funktiossa luodaan kaksi jako-Intentiä ennen ensimmäisen konfigurointia, nextShareStart katkaisee ensimmäisen tarkastusalueen toisen konstruktorin kohdalta. Esimerkiksi turvallisesti konfiguroitu toinen Intent ja sen jälkeen tehty first.putExtra(Intent.EXTRA_STREAM, uri) johtavat siihen, ettei ensimmäisen Intentin puuttuvaa ClipDataa tai lukuoikeuslippua havaita lainkaan. Rajaa alue Intent-muuttujan tai sitä ympäröivän lohkon perusteella sen sijaan, että jokainen myöhempi ACTION_SEND-osuma olisi ehdoton loppuraja.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
app/src/test/java/com/dbcheck/app/ui/meter/components/MeterExpandableComponentsContractTest.kt (1)
39-47: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftAdd light- and dark-theme runtime coverage.
This source-string test cannot verify the shared header’s rendered semantics, click behavior, or theme behavior. Add Compose UI or screenshot tests that exercise the header in both light and dark themes, including expansion and accessibility semantics.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/test/java/com/dbcheck/app/ui/meter/components/MeterExpandableComponentsContractTest.kt` around lines 39 - 47, Replace the source-string assertions in sharedExpandableHeaderIsTokenizedAccessibleAndTogglesState with Compose UI or screenshot coverage for ExpandableCardHeader, exercising it under both light and dark themes. Verify rendered theme behavior, expansion changes after clicking the header, and accessibility semantics including the state description and toggle action.Source: Coding guidelines
app/src/screenshotTest/kotlin/com/dbcheck/app/ComponentScreenshotTests.kt (1)
1117-1125: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse shared spacing and radius tokens in the preview container.
ComponentPreviewContainerhardcodes16.dpand draws a square background. Use the shared spacing token and a shared radius of at least 8dp before applying the background.As per coding guidelines, “Centralize design tokens such as colors, spacing” and “All containers must have a minimum corner radius of 8dp.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/screenshotTest/kotlin/com/dbcheck/app/ComponentScreenshotTests.kt` around lines 1117 - 1125, Update ComponentPreviewContainer to replace the hardcoded 16.dp padding with the existing shared spacing token, and apply a shared corner-radius token of at least 8dp before the background modifier. Keep the existing theme surface color and container layout unchanged.Source: Coding guidelines
app/src/main/java/com/dbcheck/app/ui/settings/components/AudioCalibrationSection.kt (1)
294-308: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract a shared chip-group helper
AudioCalibrationSection.ktduplicates the same label/spacing/FlowRowstructure already used bySettingsChipGroup; move that layout into a reusable helper and use it here instead of keeping two near-identical implementations.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/main/java/com/dbcheck/app/ui/settings/components/AudioCalibrationSection.kt` around lines 294 - 308, Reuse the existing SettingsChipGroup helper for CalibrationChipControls instead of maintaining a duplicate Column, label, spacing, and FlowRow layout. Update CalibrationChipControls to pass its title and content through that shared helper, preserving the current styling and chip spacing.Source: Coding guidelines
.deepsec/matchers/sensitive-android-log.ts (1)
7-7: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
logCallStartmissesLog.wtf(...).Android's
Log.wtfis a real log-call surface — it "should be used to log a condition that should not happen," so it's just as likely to carry sensitive identifiers asLog.e, but the current alternation(?:v|d|i|w|e)never matches it.♻️ Proposed fix to include `wtf`
-const logCallStart = /\b(?:Log|android\.util\.Log)\.(?:v|d|i|w|e)\s*\(/g; +const logCallStart = /\b(?:Log|android\.util\.Log)\.(?:v|d|i|w|e|wtf)\s*\(/g;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.deepsec/matchers/sensitive-android-log.ts at line 7, Update the logCallStart matcher to include the Android Log.wtf method alongside the existing v, d, i, w, and e methods, while preserving the current matching behavior for all other supported log calls.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/screenshotTest/kotlin/com/dbcheck/app/ComponentScreenshotTests.kt`:
- Around line 964-968: Add dark-theme preview variants for
SpectralAnalysisLivePreview, SpectralAnalysisSpectrogramPreview, and
SpectralAnalysisRtaPreview using uiMode = Configuration.UI_MODE_NIGHT_YES, while
preserving each existing preview’s content and selected mode. Avoid duplicates
if matching dark previews already exist elsewhere in the file.
---
Nitpick comments:
In @.deepsec/matchers/sensitive-android-log.ts:
- Line 7: Update the logCallStart matcher to include the Android Log.wtf method
alongside the existing v, d, i, w, and e methods, while preserving the current
matching behavior for all other supported log calls.
In
`@app/src/main/java/com/dbcheck/app/ui/settings/components/AudioCalibrationSection.kt`:
- Around line 294-308: Reuse the existing SettingsChipGroup helper for
CalibrationChipControls instead of maintaining a duplicate Column, label,
spacing, and FlowRow layout. Update CalibrationChipControls to pass its title
and content through that shared helper, preserving the current styling and chip
spacing.
In `@app/src/screenshotTest/kotlin/com/dbcheck/app/ComponentScreenshotTests.kt`:
- Around line 1117-1125: Update ComponentPreviewContainer to replace the
hardcoded 16.dp padding with the existing shared spacing token, and apply a
shared corner-radius token of at least 8dp before the background modifier. Keep
the existing theme surface color and container layout unchanged.
In
`@app/src/test/java/com/dbcheck/app/ui/meter/components/MeterExpandableComponentsContractTest.kt`:
- Around line 39-47: Replace the source-string assertions in
sharedExpandableHeaderIsTokenizedAccessibleAndTogglesState with Compose UI or
screenshot coverage for ExpandableCardHeader, exercising it under both light and
dark themes. Verify rendered theme behavior, expansion changes after clicking
the header, and accessibility semantics including the state description and
toggle action.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 8d344422-0438-438f-b4a2-83f535231513
⛔ Files ignored due to path filters (1)
.deepsec/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (31)
.deepsec/.gitignore.deepsec/matchers/android-exported-component.ts.deepsec/matchers/android-uri-share-without-clipdata.ts.deepsec/matchers/fileprovider-broad-path.ts.deepsec/matchers/foreground-audio-service-start.ts.deepsec/matchers/health-connect-sensitive-flow.ts.deepsec/matchers/security-matchers.test.ts.deepsec/matchers/sensitive-android-log.ts.deepsec/matchers/utils.ts.deepsec/package.json.deepsec/pnpm-workspace.yaml.deepsec/scripts/recover-processing-locks.mjs.deepsec/scripts/recover-processing-locks.test.mjsapp/gradle.lockfileapp/src/main/java/com/dbcheck/app/ui/analytics/components/SpectralStatPill.ktapp/src/main/java/com/dbcheck/app/ui/history/detail/SessionDetailScreen.ktapp/src/main/java/com/dbcheck/app/ui/meter/components/ExpandableCardHeader.ktapp/src/main/java/com/dbcheck/app/ui/meter/components/LiveActivityCard.ktapp/src/main/java/com/dbcheck/app/ui/meter/components/SoundReferenceCard.ktapp/src/main/java/com/dbcheck/app/ui/settings/components/AudioCalibrationSection.ktapp/src/main/res/values-fi/strings.xmlapp/src/main/res/values/strings.xmlapp/src/screenshotTest/kotlin/com/dbcheck/app/ComponentScreenshotTests.ktapp/src/test/java/com/dbcheck/app/ui/hearing/HearingViewModelTest.ktapp/src/test/java/com/dbcheck/app/ui/meter/components/MeterExpandableComponentsContractTest.ktapp/stability/app-debug.stabilityapp/stability/app-release.stabilitybuildscript-gradle.lockfileconfig/dependency-check-suppressions.xmlgradle/libs.versions.tomlgradle/verification-metadata.xml
💤 Files with no reviewable changes (2)
- app/src/main/res/values-fi/strings.xml
- app/src/main/res/values/strings.xml
|
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary by CodeRabbit