Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[apex] ApexCrudViolation: Recommend USER MODE, discourage WITH SECURITY ENFORCED #4368

Open
pozil opened this issue Jan 27, 2023 · 8 comments
Open

[apex] ApexCrudViolation: Recommend USER MODE, discourage WITH SECURITY ENFORCED #4368

pozil opened this issue Jan 27, 2023 · 8 comments
Labels
an:enhancement An improvement on existing features / rules

Comments

@pozil
Copy link
Contributor

pozil commented Jan 27, 2023 •
edited by adangel
Loading

Rule: ApexCRUDViolation

Let's consider the following improvements (quoting @rsoesemann here):

  • Change CRUD/FLS rules in Dml and SQOL to recommend USER MODE over all the suboptimal old solutions
  • Discourage the use of WITH_SECURITY ENFORCED as Daniel Ballinger and Chris Peterson said on TDX23
  • Check what else is missing in PMD regarding Security (e.g.with inherited sharing is also not parsed correctly)
@pozil pozil added the a:bug PMD crashes or fails to analyse a file. label Jan 27, 2023
@pozil pozil changed the title [Apex] Update parser to support new 'as user' syntax (User Mode for Database Operations) [Apex] Update parser to support new 'as user' keywords (User Mode for Database Operations) Jan 27, 2023
@adangel adangel changed the title [Apex] Update parser to support new 'as user' keywords (User Mode for Database Operations) [apex] Update parser to support new 'as user' keywords (User Mode for Database Operations) Jan 30, 2023
@adangel adangel mentioned this issue Feb 3, 2023
Closed
@rsoesemann rsoesemann mentioned this issue Mar 19, 2023
Closed
@rsoesemann rsoesemann added the an:enhancement An improvement on existing features / rules label Mar 19, 2023
@rsoesemann

This comment was marked as outdated.

Sign in to view
@pozil

This comment was marked as off-topic.

Sign in to view
@avesolovksyy

This comment was marked as outdated.

Sign in to view
@adangel adangel mentioned this issue Jun 24, 2023
Closed
@adangel
Copy link
Member

adangel commented Jun 24, 2023

For updating jorje, there is already #3973. I'll update the description of this issue to improve the rule ApexCrudViolations only.

@adangel adangel changed the title [apex] Update parser to support new 'as user' keywords (User Mode for Database Operations) [apex] ApexCrudViolation: Recommend USER MODE, discourage WITH SECURITY ENFORCED Jun 24, 2023
@adangel adangel removed the a:bug PMD crashes or fails to analyse a file. label Jun 24, 2023
adangel added a commit to adangel/pmd that referenced this issue Jun 24, 2023
@adangel
[apex] ApexCRUDViolation: Add support for "as user"
1a4d1bd 
Refs pmd#4368
@adangel adangel mentioned this issue Jun 24, 2023
Merged
4 tasks
adangel added a commit to adangel/pmd that referenced this issue Jun 24, 2023
@adangel
[apex] ApexCRUDViolation: Add support for "as user"
26c59e0 
Refs pmd#4368
adangel added a commit to adangel/pmd that referenced this issue Jun 24, 2023
@adangel
[apex] ApexCRUDViolation: Add support for "as user"
0b9d5ec 
Refs pmd#4368
@rsoesemann
Copy link
Member

rsoesemann commented Jul 20, 2023 •
edited
Loading

FYI @FishOfPrey @capeterson this is a Security related issue. @pozil already describe what needs to be changed to existing rules.

@jorgesolebur
Copy link

Hello, I am a bit confused because I can see some commits and merged over this ticket, but has this request been applied to PMD? I recently installed las version of PMD (7.0-rc3) and in DML operations using "insert as user" still throws an exception in PMD

@adangel
Copy link
Member

adangel commented Sep 30, 2023

"insert as user" still throws an exception in PMD

This has been fixed with #3973 which will be part of the rc4 coming out soon (today/tomorrow).

Apart from that, this ticket here (I mean #4368) has not been addressed and is still open.

@nwcm
Copy link
Contributor

nwcm commented Dec 11, 2023

Previous to rc4 and support for as user|system

We required all DML to be via database. methods.

I still think we will stick with this even with the new support for consistency.

    <rule name="NoDMLStatement" language="apex" message="Use Database methods" class="net.sourceforge.pmd.lang.rule.XPathRule">
        <description>Database.insert(foo, System.AccessLevel.USER_MODE)</description>
        <priority>1</priority>
        <properties>
            <property name="version" value="3.1"/>
            <property name="xpath">
                <value>
                    <![CDATA[
                //DmlInsertStatement|//DmlUpsertStatement|//DmlDeleteStatement|//DmlUpdateStatement|//DmlUndeleteStatement
                    ]]>
                </value>
            </property>
        </properties>
    </rule>
    <rule name="databaseMethodsHaveAccessLevel" language="apex" message="Database methods must pass AccessLevel and use of SYSTEM_MODE requires comment // CRUD/FLS" class="net.sourceforge.pmd.lang.rule.XPathRule">
        <priority>1</priority>
        <properties>
            <property name="version" value="3.1"/>
            <property name="xpath">
                <value>
            <![CDATA[
            //MethodCallExpression[
                    replace(lower-case(@FullMethodName),'system.','') = (
                        "search.query",
                        "search.find",
                        "search.suggest",
                        "database.convertlead",
                        "database.countquery",
                        "database.countquerywithbinds",
                        "database.delete",
                        "database.deleteasync",
                        "database.deleteimmediate",
                        "database.getquerylocator",
                        "database.getquerylocatorwithbinds",
                        "database.insert",
                        "database.insertasync",
                        "database.insertimmediate",
                        "database.merge",
                        "database.query",
                        "database.querywithbinds",
                        "database.undelete",
                        "database.update",
                        "database.upsert",
                        "database.updateasync",
                        "database.updateimmediate"
                    )]
            [not(VariableExpression[lower-case(@Image)=('user_mode','system_mode')]/ReferenceExpression[lower-case(@Image)=('accesslevel','system')])]
            ]]>
            </value>
            </property>
        </properties>
    </rule>

@rsoesemann rsoesemann mentioned this issue May 7, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
an:enhancement An improvement on existing features / rules
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@adangel @pozil @rsoesemann @jorgesolebur @avesolovksyy @nwcm