Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 20 additions & 4 deletions cli/src/cordova.ts
Original file line number Diff line number Diff line change
Expand Up @@ -867,14 +867,30 @@ export async function writeCordovaAndroidManifest(
});
});
const cleartextString = 'android:usesCleartextTraffic="true"';
const cleartextValue =
(cleartext || config.app.extConfig.server?.cleartext) && !applicationXMLAttributes.includes(cleartextString)
? cleartextString
const networkSecurityConfigValue =
cleartext || config.app.extConfig.server?.cleartext || applicationXMLAttributes.includes(cleartextString)
? 'android:networkSecurityConfig="@xml/network_security_config"'
: '';
const networkSecurityConfigDir = join(config.android.cordovaPluginsDirAbs, 'src', 'main', 'res', 'xml');

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Now that I see it, I'm skeptical of this working in general for Capacitor 9.

Since capacitor 9 only comes with the cordovaPluginsDirAbs tied to the app project if the app has cordova plugins, we could run into situations where setting config.app.extConfig.server?.cleartext would have no effect because the app has no Cordova Plugins - which would lead me to indicate that perhaps this needs to be written somewhere else?

This is kind of separate from your PR, but I guess is highlighted by it.

Did you run into this when testing, or did you always test in apps with a Cordova Plugin?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, ran into it during testing. Started in an app without any Cordova plugins with server.cleartext=true set, and on the first sync noticed the mechanism wasn't triggered, writeCordovaAndroidManifest only runs with effect when there are Cordova plugins (via the if(enableCordova) path in update.ts). Without one, the library manifest stayed empty (no network-security-config, no attribute), and server.cleartext=true was a no-op. Ended up installing cordova-plugin-device so the mechanism would trigger and I could test in runtime.

From what I understand this is inherited from the pre-existing behavior, the old usesCleartextTraffic="true" injection had the same limitation on Cap 9, but this PR keeps it that way.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yes you are correct that this is not a fault with your PR specifically, but it should be fixed for Cap 9 imo.

Now, that doesn't mean it needs to be fixed in your PR necessarily, but maybe because of the other comment it could need further changes? We can align offline as well.

const networkSecurityConfigPath = join(networkSecurityConfigDir, 'network_security_config.xml');
if (networkSecurityConfigValue) {
await ensureDir(networkSecurityConfigDir);
await writeFile(
networkSecurityConfigPath,
`<?xml version="1.0" encoding="utf-8"?>
<network-security-config>
<base-config cleartextTrafficPermitted="true" />
</network-security-config>
`,
);
} else if (await pathExists(networkSecurityConfigPath)) {
await remove(networkSecurityConfigPath);
}
Comment on lines +876 to +888

@OS-pedrogustavobilro OS-pedrogustavobilro Aug 14, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Because se are not writing to the app's manifest directly (but maybe we might need to, in relation to my other comment), I think we're fine. However, I wonder if you tested this in an app that has their own network-security-config (maybe with clearTestTraffic=false, or with pinning configuration like SSL Pinning Plugin).

Wondering if those could clash and fail the build (which may not fail before with the deprecated way), and if we need to place additional guards to protect those cases.

Let me know if that makes sense.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Tested with server.cleartext=true + app declaring its own network-security-config, in order to understand what could happen, and here are the findings:

  • Same filename (network_security_config.xml): build passes, but the app's file wins, our server.cleartext=true is silently ignored. Also tested with: config false + app network_security_config true still allows HTTP.

  • Different filename: build fails with manifest merger conflict (different values for android:networkSecurityConfig between library and app). The error itself suggests the standard Android fix: add tools:replace="android:networkSecurityConfig" to the app's <application>, once the user does that, build passes and the app's file wins.

Manifest merger failed : Attribute application@networkSecurityConfig value=(@xml/my_security_config) from AndroidManifest.xml:12:9-64 is also present at [:capacitor-cordova-android-plugins] AndroidManifest.xml:8:18-78 value=(@xml/network_security_config). Suggestion: add 'tools:replace="android:networkSecurityConfig"' to <application> element at AndroidManifest.xml:5:5-39:19 to override.

So when the app has its own NSC, our injection is either silently overridden or breaks the build.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hmm, that's why we may need to write the config directly to the app, but in a way that doesn't break the existing config (I'm fine with overriding the cleartext to true if the config requests but the app has it at false, which is technically what the CLI is meant to be doing, but not so much overwriting the entire networkSecurityConfig file).

But also that has implications in relation to the other PR comment.


let content = `<?xml version='1.0' encoding='utf-8'?>
<manifest xmlns:android="http://schemas.android.com/apk/res/android"
xmlns:amazon="http://schemas.amazon.com/apk/res/android">
<application ${applicationXMLAttributes.join('\n')} ${cleartextValue}>
<application ${applicationXMLAttributes.filter((attr) => !attr.includes('android:usesCleartextTraffic')).join('\n')} ${networkSecurityConfigValue}>
${applicationXMLEntries.join('\n')}
</application>
${rootXMLEntries.join('\n')}
Expand Down
Loading