-
Notifications
You must be signed in to change notification settings - Fork 0
[faq] introduce insecure build #127
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
Conversation
Deploying docs with
|
Latest commit: |
1daf4b8
|
Status: | ✅ Deploy successful! |
Preview URL: | https://35cf96e2.docs-7wm.pages.dev |
Branch Preview URL: | https://faq-introduce-insecure-versi.docs-7wm.pages.dev |
WalkthroughDocumentation updates introduce an official insecure APK build variant for local HTTP webhook testing, remove prior manual rebuild instructions, add Build Variants and Permissions guidance in installation, adjust installation steps and headers, and add security considerations and See Also links. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (2)
docs/installation.md (1)
29-40
: Consider adding download integrity & variant-name details
The new Build Variants section is clear, but two practical details are missing:
- Integrity verification – users are sideloading an APK; a short note (or link) on verifying the APK’s SHA-256 signature would prevent supply-chain surprises.
- Exact artifact names – you mention “Standard (Secure) Build”, but only list
app-insecure.apk
. Stating the filename of the secure build (e.g.app-release.apk
) avoids ambiguity and helps automated scripts.- To download the insecure build, visit … + To download the insecure build, visit … + + **Verify the download** + + 1. Compare the SHA-256 checksum against the value published on the release page. + ```bash + sha256sum app-insecure.apk + ``` + 2. Only continue if the hashes match.Optional but improves trustworthiness and usability.
docs/faq/webhooks.md (1)
101-126
: Tighten cross-references & reuse existing anchors
Great addition, but you can remove duplication and improve navigation:
- Step 3 currently links to the whole installation doc. Linking directly to the Build Variants subsection makes it easier to spot the insecure build instructions.
- Consider reusing the term “Standard (Secure) Build” exactly as defined in
installation.md
for wording consistency.-3. Install following the [standard installation process](../installation.md) +3. Install following the + [Build Variants → Installing from APK](../installation.md#build-variants) + instructions(Confirm the anchor slug generated by MkDocs; typically it will be
#build-variants
.)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
docs/faq/webhooks.md
(1 hunks)docs/installation.md
(3 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/installation.md
[style] ~37-~37: Using many exclamation marks might seem excessive (in this case: 3 exclamation marks for a text that’s 1831 characters long)
Context: ...nd select the app-insecure.apk
file. !!! warning "Security Notice" The insec...
(EN_EXCESSIVE_EXCLAMATION)
docs/faq/webhooks.md
[style] ~121-~121: Using many exclamation marks might seem excessive (in this case: 21 exclamation marks for a text that’s 3667 characters long)
Context: ...tallation process](../installation.md) !!! note "Security Considerations" - Al...
(EN_EXCESSIVE_EXCLAMATION)
e9baae0
to
36cef45
Compare
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.
Actionable comments posted: 0
🧹 Nitpick comments (3)
docs/installation.md (1)
29-37
: Add cross-reference to FAQ to avoid duplicated maintenanceThe “Build Variants” subsection re-explains the insecure build that is already covered in FAQ → Using HTTP Webhooks in Local Development. Adding a short “See also” link will keep both pages in sync and spare future edits.
- The application is available in two build variants: + The application is available in two build variants + *(See also: [Using HTTP Webhooks in Local Development](../faq/webhooks.md#using-http-webhooks-in-local-development))*:docs/faq/webhooks.md (2)
101-101
: Remove trailing colon to satisfy MD026Markdown-lint flags punctuation at the end of headings. Dropping the colon keeps the heading clean and avoids CI/style noise.
-## Using HTTP Webhooks in Local Development :material-test-tube: +## Using HTTP Webhooks in Local Development :material-test-tube
105-126
: Consider linking back to Installation page for download stepsThe section nicely explains when and why to use the insecure build, but readers must scroll for installation details. A direct link streamlines navigation and reduces duplicated instructions.
Example:
### How to Obtain See the *Installation → Build Variants* section for the full download & install steps, or: 1. …
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
docs/faq/webhooks.md
(1 hunks)docs/installation.md
(3 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/faq/webhooks.md
[style] ~121-~121: Using many exclamation marks might seem excessive (in this case: 21 exclamation marks for a text that’s 3665 characters long)
Context: ...tallation process](../installation.md) !!! note "Security Considerations" - Al...
(EN_EXCESSIVE_EXCLAMATION)
docs/installation.md
[style] ~37-~37: Using many exclamation marks might seem excessive (in this case: 3 exclamation marks for a text that’s 1831 characters long)
Context: ...nd select the app-insecure.apk
file. !!! warning "Security Notice" The insec...
(EN_EXCESSIVE_EXCLAMATION)
🪛 markdownlint-cli2 (0.17.2)
docs/faq/webhooks.md
101-101: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cloudflare Pages
45e3723
to
968eb80
Compare
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.
Actionable comments posted: 0
🧹 Nitpick comments (2)
docs/faq/webhooks.md (2)
101-101
: Heading violates MD026 – trailing punctuation colon
Markdown-lint flags the colon that closes the:material-test-tube:
icon as “trailing punctuation”. Add a rule-disable comment or move the icon to the front to silence CI lint noise.-## Using HTTP Webhooks in Local Development :material-test-tube: +<!-- markdownlint-disable-next-line MD026 --> +## :material-test-tube: Using HTTP Webhooks in Local Development
105-110
: Clarify scope of “local network” to avoid mis-use
Stating “local network deployments” might mislead users into thinking any RFC1918 address is automatically safe. Consider adding a sentence that the build disables clear-text restrictions for all hosts, not just those on private subnets, and that traffic may still traverse untrusted segments (e.g., public Wi-Fi).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
docs/faq/webhooks.md
(1 hunks)docs/installation.md
(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/installation.md
🧰 Additional context used
🪛 LanguageTool
docs/faq/webhooks.md
[style] ~121-~121: Using many exclamation marks might seem excessive (in this case: 21 exclamation marks for a text that’s 3665 characters long)
Context: ...tallation process](../installation.md) !!! note "Security Considerations" - Al...
(EN_EXCESSIVE_EXCLAMATION)
🪛 markdownlint-cli2 (0.17.2)
docs/faq/webhooks.md
101-101: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
🔇 Additional comments (1)
docs/faq/webhooks.md (1)
121-125
: False-positive “excessive exclamation marks” warning can be ignored
The triple-bangs belong to MkDocs admonition syntax, not exuberant punctuation. No action needed unless the docs pipeline treats it as an error; if so, suppress the rule for this block.
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.
Actionable comments posted: 1
🧹 Nitpick comments (5)
docs/faq/webhooks.md (5)
88-88
: Silence MD026 for headings using icon syntax.markdownlint counts the trailing colon from the :material-...: icon as trailing punctuation. Add an inline disable for MD026 above this heading.
+<!-- markdownlint-disable-next-line MD026 --> ## How to manage webhooks for specific devices? :material-cellphone-link:
101-101
: Silence MD026 for iconized heading here as well.Same trailing-colon issue due to :material-test-tube:. Add an inline disable.
+<!-- markdownlint-disable-next-line MD026 --> ## Using HTTP Webhooks in Local Development :material-test-tube:
103-107
: Strengthen the admonition severity to reflect risk.Use a danger admonition to clearly communicate the risk level for the insecure build.
-!!! warning "Production Use Prohibited" +!!! danger "Production Use Prohibited" This build **MUST NOT** be used in public environments as it disables critical security protections. It is strictly for local development and deployment on trusted networks.
103-104
: Call out the exception to the HTTPS requirement for consistency.Explicitly reference that this is the only exception to the HTTPS-only guidance above to avoid confusion.
-We provide an **insecure build variant** that allows HTTP webhook endpoints for local network deployments. +We provide an **insecure build variant** that allows HTTP webhook endpoints for local network deployments. + +!!! note "Exception to HTTPS Requirement" + This is the only exception to the HTTPS-only policy described above. Use it strictly for local development.
118-126
: LanguageTool warning is a false positive.The “excessive exclamation marks” warning stems from MkDocs admonition syntax (
!!!
). Consider suppressing this rule for docs or configure LanguageTool to ignore!!!
at line starts.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
docs/faq/webhooks.md
(2 hunks)docs/installation.md
(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/installation.md
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
docs/faq/webhooks.md
88-88: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
101-101: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
🪛 LanguageTool
docs/faq/webhooks.md
[style] ~121-~121: Using many exclamation marks might seem excessive (in this case: 21 exclamation marks for a text that’s 3665 characters long)
Context: ...tallation process](../installation.md) !!! note "Security Considerations" - Al...
(EN_EXCESSIVE_EXCLAMATION)
🔇 Additional comments (1)
docs/faq/webhooks.md (1)
122-126
: Verify Android insecure build cleartext scope and update docsWe didn’t find any
network_security_config.xml
orusesCleartextTraffic
/networkSecurityConfig
entries in the repo, so please confirm how the “insecure” build actually enables HTTP (globally vs. only for RFC1918). Once confirmed, update the note to:
- Clearly state the exact scope of allowed cleartext (e.g. global or limited to 10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16).
- Add explicit revert steps for rolling back to the secure build.
- Advise disabling auto-updates for the insecure build to avoid accidental long-term use.
Proposed doc diff:
!!! note "Security Considerations" - Always revert to the standard (secure) build for production use - Never expose insecure builds to public networks - - This build bypasses Android's cleartext traffic restrictions + - This build bypasses Android's cleartext traffic restrictions + - HTTP is allowed only for private RFC1918 ranges (e.g., 10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16); public IPs/domains still require HTTPS + - Revert steps: + 1. Backup any local data if needed + 2. Uninstall the insecure build + 3. Install the secure build from Releases + - Disable auto-updates for this insecure build to prevent unintended extended use
b7b8573
to
c4d9a4d
Compare
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.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
docs/installation.md (1)
17-27
: Modernize install steps and add integrity/authenticity verification
- On Android 8.0+, the setting is per‑app (“Install unknown apps”) rather than a global “Unknown sources.”
- Add checksum and signature checks before installation.
- Mention choosing the correct build variant if multiple APKs are offered.
1. Visit the [Releases page](https://github.com/capcom6/android-sms-gateway/releases) on GitHub. -2. Select and download the most recent APK. +2. Select and download the appropriate APK (see “Build Variants” below). +3. Verify the APK’s SHA‑256 checksum against the value published in the release notes: + - macOS/Linux: `shasum -a 256 <apk-file.apk>` + - Windows (PowerShell): `Get-FileHash <apk-file.apk> -Algorithm SHA256` +4. Verify the APK signature is from the official signing key: + - With Android SDK: `apksigner verify --print-certs <apk-file.apk>` and compare the certificate SHA‑256 fingerprint to the published official fingerprint. -3. Move the APK file to your Android device. -4. Access **Settings** > **Security** or **Privacy** on your device. -5. Activate **Unknown sources** to allow installations from non-Play Store sources. -6. Use a file manager to find the APK file on your device. -7. Tap the APK to begin installation. -8. Complete the setup as prompted on-screen. +5. Move the APK file to your Android device (if downloaded on desktop). +6. On Android 8.0+ go to **Settings** > **Apps & notifications** > **Special app access** > **Install unknown apps**, then allow your installer app (e.g., Files, browser) to install unknown apps. On older versions, enable **Unknown sources** under **Security**. +7. Use a file manager to find the APK on your device. +8. Tap the APK to begin installation. +9. Complete the setup as prompted on-screen.I can provide a reusable snippet documenting the official signing certificate fingerprint to reference here and in release notes.
♻️ Duplicate comments (1)
docs/faq/webhooks.md (1)
116-118
: Add checksum and signature verification before installing the APKIntegrity and identity verification steps are essential when distributing an “insecure” build to reduce supply‑chain risk and avoid mixing builds. This mirrors prior feedback.
1. Visit the [GitHub Releases](https://github.com/capcom6/android-sms-gateway/releases) page 2. Download the `app-insecure.apk` file -3. Install following the [standard installation process](../installation.md) +3. Verify the APK checksum from the release notes: + - Compare the published SHA-256 with a locally computed one: + - macOS/Linux: `shasum -a 256 app-insecure.apk` + - Windows (PowerShell): `Get-FileHash app-insecure.apk -Algorithm SHA256` +4. Verify the APK signature is from the official signing key: + - With Android SDK: `apksigner verify --print-certs app-insecure.apk` and compare the certificate SHA-256 fingerprint against the documented official fingerprint. +5. Install following the [standard installation process](../installation.md)Would you like me to provide wording to publish the official cert fingerprint in docs/installation.md for users to compare against?
🧹 Nitpick comments (3)
docs/faq/webhooks.md (3)
86-86
: Fix markdownlint MD026: heading ends with punctuation due to icon tokenMaterial icon shortcodes at the end trigger MD026. Move the icon before the text to satisfy the linter.
-## How to manage webhooks for specific devices? :material-cellphone-link: +## :material-cellphone-link: How to manage webhooks for specific devices?
99-99
: Fix markdownlint MD026: heading ends with punctuation due to icon tokenSame issue here. Move the icon before the text.
-## Using HTTP Webhooks in Local Development :material-test-tube: +## :material-test-tube: Using HTTP Webhooks in Local Development
103-105
: Strengthen the security warning with concrete risksConsider calling out common risks explicitly (e.g., unencrypted SMS payloads susceptible to interception/mitM on shared networks, compliance exposure) and recommend isolating devices on trusted VLANs and firewalled subnets.
!!! warning "Production Use Prohibited" - This build **MUST NOT** be used in public environments as it disables critical security protections. It is strictly for local development and deployment on trusted networks. + This build **MUST NOT** be used in public environments as it disables critical security protections. It is strictly for local development on trusted, firewalled networks. Data sent over HTTP is unencrypted and can be intercepted or modified on shared networks (e.g., Wi‑Fi). Isolate test devices (e.g., dedicated VLAN) and avoid any internet exposure.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
docs/faq/webhooks.md
(2 hunks)docs/installation.md
(3 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
docs/faq/webhooks.md
86-86: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
99-99: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
🪛 LanguageTool
docs/faq/webhooks.md
[style] ~119-~119: Using many exclamation marks might seem excessive (in this case: 21 exclamation marks for a text that’s 3770 characters long)
Context: ...tallation process](../installation.md) !!! note "Security Considerations" - Al...
(EN_EXCESSIVE_EXCLAMATION)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (7)
docs/faq/webhooks.md (3)
83-85
: Minor: wording tweak looks fineThe reflow/spacing of the Tradeoff note is fine; no issues.
101-101
: Clarify the scope of what “insecure build variant” relaxesPlease clarify whether HTTP is allowed only for webhook transport or for all outbound/inbound network calls. Explicit scope reduces misunderstandings and helps users assess risk.
106-113
: “When to Use” list is clearThe examples are appropriate and scoped to local development. No changes needed.
docs/installation.md (4)
1-1
: LGTM on title stylingEmoji‑prefixed “Installation” header reads well.
5-5
: Prerequisites section looks goodConcise and clear.
38-45
: Cross-references look goodThe ordering and See Also links align with the new variant flow.
29-37
: Clarify build variant identities and artifact details– Standard (Secure) Build: Recommended for most users. Enforces HTTPS for all communications (except
127.0.0.1
).
Artifact: typicallyapp-release.apk
.
– Insecure Build: For local development only. Allows HTTP webhook endpoints on local networks. Not recommended for production use.
Artifact: typicallyapp-insecure.apk
.
Package name (applicationId): please confirm if this differs from the secure build.See also: Using HTTP Webhooks in Local Development and Security Notes.
Please verify the exact APK filename patterns and whether the insecure build uses a distinct applicationId to avoid side-by-side confusion.
8a21a41
to
1daf4b8
Compare
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.
Actionable comments posted: 0
♻️ Duplicate comments (3)
docs/faq/webhooks.md (3)
120-120
: LanguageTool “excessive exclamation marks” is a false positiveThe “!!!” is MkDocs admonition syntax, not punctuation. Consider suppressing this rule in tooling for docs or per-file.
116-118
: Add checksum and signature verification before installRelease assets should be integrity- and identity-checked to prevent supply-chain mixups.
1. Visit the [GitHub Releases](https://github.com/capcom6/android-sms-gateway/releases) page 2. Download the `app-insecure.apk` file -3. Install following the [standard installation process](../installation.md) +3. Verify the APK checksum from the release notes: + - macOS/Linux: `shasum -a 256 app-insecure.apk` + - Windows (PowerShell): `Get-FileHash app-insecure.apk -Algorithm SHA256` + - Compare against the published SHA-256 +4. Verify the APK is signed by the official signing key: + - `apksigner verify --print-certs app-insecure.apk` or `keytool -printcert -jarfile app-insecure.apk` + - Confirm the SHA-256 certificate fingerprint matches the official signer (see “Release Integrity” docs) +5. Install following the [standard installation process](../installation.md)If you want, I can draft a central “Release Integrity” doc and link it here and in Installation.
120-123
: Document package identity and signer fingerprint in Security ConsiderationsMake the insecure build easy to identify and uninstall; publish signer fingerprint once and link.
!!! note "Security Considerations" - Always revert to the standard (secure) build for production use - Never expose insecure builds to public networks - This build bypasses Android's cleartext traffic restrictions + - Package name (applicationId): com.example.android_sms_gateway.insecure (replace with the actual value) + - Signed by: Official release key (SHA-256 fingerprint: <link to authoritative fingerprint>) + - Uninstall the insecure build after testing to avoid accidental useI can add the exact applicationId and wire this to the central “Release Integrity” page if you share the values.
🧹 Nitpick comments (4)
docs/faq/webhooks.md (4)
86-86
: Suppress MD026 for heading with Material iconmarkdownlint misinterprets the trailing colon from the icon as punctuation. Suppress locally.
-## How to manage webhooks for specific devices? :material-cellphone-link: +<!-- markdownlint-disable-next-line MD026 --> +## How to manage webhooks for specific devices? :material-cellphone-link:
99-101
: Add lint suppression and cross-link Build Variants
- MD026: same icon-related false positive; suppress it.
- Also link readers to Build Variants in Installation for consistency.
-## Using HTTP Webhooks in Local Development :material-test-tube: +<!-- markdownlint-disable-next-line MD026 --> +## Using HTTP Webhooks in Local Development :material-test-tube: -We provide an **insecure build variant** that allows HTTP webhook endpoints for local network deployments. +We provide an **insecure build variant** that allows HTTP webhook endpoints for local network deployments. See also: [Build Variants](../installation.md#build-variants).
103-105
: Strengthen the warning with concrete guardrailsMake explicit that only test devices and non-production data should be used, and ensure the UI clearly labels the build.
!!! warning "Production Use Prohibited" - This build **MUST NOT** be used in public environments as it disables critical security protections. It is strictly for local development and deployment on trusted networks. + This build **MUST NOT** be used in public environments as it disables critical security protections. It is strictly for local development and deployment on trusted networks. + - Install only on non-production test devices + - Treat all webhook payloads as plaintext on the wire; do not use with real user data + - The app UI should clearly label "Insecure build" to avoid confusion during testing
108-113
: Minor: Call out scope and device policyOptional clarity: emphasize scope and test-only policy in “When to Use”.
- Testing webhook integrations on local networks (e.g., `192.168.0.100:9876`) - Developing without a valid SSL certificate - Needing to test HTTP endpoints during development +- Use only on isolated developer networks and test devices; never on production fleets
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
docs/faq/webhooks.md
(2 hunks)docs/installation.md
(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/installation.md
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
docs/faq/webhooks.md
86-86: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
99-99: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
🪛 LanguageTool
docs/faq/webhooks.md
[style] ~119-~119: Using many exclamation marks might seem excessive (in this case: 21 exclamation marks for a text that’s 3770 characters long)
Context: ...tallation process](../installation.md) !!! note "Security Considerations" - Al...
(EN_EXCESSIVE_EXCLAMATION)
🔇 Additional comments (1)
docs/faq/webhooks.md (1)
83-85
: LGTM on the Tradeoff note tweakWhitespace/formatting around the admonition looks fine.
Summary by CodeRabbit