WeTransfer / Diagnostics

Allow users to easily share Diagnostics with your support team to improve the flow of fixing bugs.
MIT License
933 stars 53 forks source link

Missing API declaration #168

Closed dneykov closed 2 months ago

dneykov commented 3 months ago

Hi,

first I want to mention that is great package.

Now to the problem. After my last update submitted to the store I get an email with issues regarding Missing API declaration https://developer.apple.com/documentation/bundleresources/privacy_manifest_files/describing_use_of_required_reason_api

Below is the list with APIs that needs declaration:

Beside NSPrivacyAccessedAPICategoryUserDefaults I don't think I'm using the other APIs in my code. I thought they might be used for the diagnostics. Do you plan to include PrivacyInfo.xcprivacy or I need to add it to my app?

Thanks.

edorphy commented 3 months ago

Same as #157

ArturoLee commented 3 months ago

+1

ArturoLee commented 2 months ago

Currently working on a fix: https://github.com/ArturoLee/Diagnostics/tree/privacyManifest

However, I didn't find any usage of NSPrivacyAccessedAPICategoryFileTimestamp or NSPrivacyAccessedAPICategorySystemBootTime in this library. Can you confirm this @AvdLee

Is it possible you may be using other libraries that are missing those declerations? @dneykov

dneykov commented 2 months ago

@ArturoLee you might be right. Some other packages could use these APIs as well. I thought could be Diagnostics package since it's checking some system preferences etc.

I already notice that NSPrivacyAccessedAPICategorySystemBootTime is used by Alamofire package and they already added the declaration.

peagasilva commented 2 months ago

Hi all!

Just a heads up that we're working on it internally and will have an update soon.

For all who are wondering if a privacy manifest will be required for all 3rd party libraries, that's the case only for the commonly used libraries, and Diagnostics is not part of the list.

CleanShot 2024-04-15 at 11 08 49@2x

So if you add the privacy manifest to you app, you should be good - unless you use a 3rd party library that is commonly used and it doesn't have a privacy manifest.

Cheers!

edorphy commented 2 months ago

The problems arise with how you, the developer, generate reports and send them off device.

Even for user defaults, there isn't an approved usage for the smart insights of user defaults, to say more directly, you cannot call dictionary representation on user defaults and send that data off device. There is not even an approved reason of "for optional bug request" like the disk storage reasons.

Even if you override the smart insights, the code will probably be flagged by Apple's code scan or usage API tooling (unverified).

For example, if you declare CA92.1, you're still not compliant because you're reading data that Apple placed in User Defaults. Furthermore you're transmitting it off device.

While yes the framework is not required to specify a privacy manifest themselves, that puts the pressure on everyone using the library. Then each developer needs to know how to answer the questions and interpret the third party library for its capabilities with respect to the ITMS warnings an app received.

peagasilva commented 2 months ago

Implemented in https://github.com/WeTransfer/Diagnostics/pull/171.

peagasilva commented 2 months ago

Hi @edorphy, thanks for chipping in!

The problems arise with how you, the developer, generate reports and send them off device.

I'm not entirely sure on what you meant here, or what you are addressing. Could you please elaborate?

Even for user defaults, there isn't an approved usage for the smart insights of user defaults, to say more directly, you cannot call dictionary representation on user defaults and send that data off device. There is not even an approved reason of "for optional bug request" like the disk storage reasons.

Even if you override the smart insights, the code will probably be flagged by Apple's code scan or usage API tooling (unverified).

For example, if you declare CA92.1, you're still not compliant because you're reading data that Apple placed in User Defaults. Furthermore you're transmitting it off device.

You are correct in parts. In the way UserDefaultsReporter was implemented before, changes were needed in order to comply with the required reason. In order to minimize the breaking changed to the users of the library, we went with a wrapper approach, using C56D.1 - implemented in https://github.com/WeTransfer/Diagnostics/pull/171, as we cannot use either CA92.1 or 1C8F.1.

image

While yes the framework is not required to specify a privacy manifest themselves, that puts the pressure on everyone using the library. Then each developer needs to know how to answer the questions and interpret the third party library for its capabilities with respect to the ITMS warnings an app received.

Yes, that's is correct and any code that developers include in their codebases should ultimately be the developer responsibility (i.e. if there's a library they use that doesn't contain a privacy manifest, it's their responsibility to audit the code and create a privacy manifest to the app), ourselves included.