fix: added squelch and no-pii annotation check by Akanshu-2u · Pull Request #551 · openedx/edx-lint

Akanshu-2u · 2026-07-07T06:40:37Z

Description:

PII variables (e.g. email, username, full_name) could silently leak into logs, print statements, or exceptions with no automated guardrail. Stale .. no_pii: annotations on Django models also went undetected.

Solution:

Replaced vague words (name, ip, image) with specific ones (full_name, ip_address, profile_image) so common variable names don't trigger false alerts.
Added 21 known-safe field names (e.g., service_username, email_enabled, attr_full_name, location) to an ignore list so they are always skipped.
Updated the same lists in 3 places: the plugin code, the runtime defaults, and the pylintrc template — so nothing goes out of date.
New test cases lock in this behavior so these false positives can never come back.
A field literally named email (but it's actually a BooleanField) still flags. Developers just need to add one inline comment to silence it:

python
# pylint: disable=pii-invalid-no-pii-annotation

Private JIRA Ticket:

BOMS-587

ttak-apphelix · 2026-07-15T10:26:01Z

+
+# Comma-separated list of identifier substrings treated as likely PII.
+# Substring matching is used, so 'email' will match 'user_email_address'.
+pii-terms=email,secondary_email,username,retired_username,password,full_name,first_name,last_name,phone,phone_number,birth_date,ip_address,location,address,mailing_address,gender,profile_image,job_title,social_link


Format it for better readability.

ttak-apphelix · 2026-07-15T10:28:55Z

+
+# Comma-separated list of identifier substrings treated as likely PII.
+# Substring matching is used, so 'email' will match 'user_email_address'.
+pii-terms=email,secondary_email,username,retired_username,password,full_name,first_name,last_name,phone,phone_number,birth_date,ip_address,location,address,mailing_address,gender,profile_image,job_title,social_link


can you recheck if retired_username is considered as PII?

rahulkanneri-2u · 2026-07-15T11:13:23Z

+class PiiAnnotationChecker(PiiConfigMixin, BaseChecker):
+    """
+    Fires ``pii-invalid-no-pii-annotation`` (W7633) when a concrete Django model
+


Nit: Remove this black line

rahulkanneri-2u · 2026-07-15T11:14:32Z

+
+

Nit: Two blank lines here, Remove it.

rahulkanneri-2u · 2026-07-15T11:18:02Z

 Unreleased
 ~~~~~~~~~~

+6.2.0 - 2026-06-26


Just double checking, This jumps straight from 5.7.0 to 6.2.0, with no 6.0.0 or 6.1.0, Is this normal?

6.0.0 and 6.1.0 were previous releases that simply weren't documented in the CHANGELOG. So 6.2.0 here is correct, the jump in the CHANGELOG is just a documentation gap from earlier releases, not a versioning error.

ttak-apphelix · 2026-07-15T10:59:28Z

+            "job_title",
+            "social_link",
+        ])
+        self._pii_terms_cache = [t.strip().lower() for t in raw_terms if t.strip()]


use identifiable name instead of t

Addressed this and related ones also.

ttak-apphelix · 2026-07-15T11:01:38Z

+            "attr_email", "default_email", "skip_email_verification",
+            "location", "_location", "example_full_name"
+        ])
+        self._safe_keys_cache = {k.strip().lower() for k in raw_keys if k.strip()}


same here. Rename K to identifiable name and other places also

Addressed this and related ones also.

vgulati-apphelix · 2026-07-15T18:35:10Z

+    default_email,
+    skip_email_verification,
+    location,
+    _location,


We should remove location/_location from this list.

Akanshu-2u · 2026-07-16T07:20:13Z

+    email,
+    secondary_email,
+    username,
+    retired_username,


Remove retired_username, as it is non-traceable and does not follow the OEP-30 validation.

Akanshu-2u · 2026-07-16T07:29:48Z

+    attr_email,
+    default_email,
+    skip_email_verification,
+    location,


Check whether it has to be part of PII-terms as its flagged as PII in OEP-30 doc.

Akanshu-2u · 2026-07-16T07:38:21Z

+_NO_PII_COMMENT_RE = re.compile(r"[\s]*#[\s]*\.\.\s*no_pii", re.IGNORECASE)
+
+# Lines above ``class`` to scan for a comment-style annotation.
+_ANNOTATION_LOOKAHEAD = 5


Increase it to 10, so that detection of pii and no_pii terms doesn't get missed out if description length increases.

Akanshu-2u · 2026-07-16T07:40:02Z

+# OEP-0030 PII identifier substrings — substring-matched against variable names.
+_DEFAULT_PII_TERMS = [
+    "email", "secondary_email",
+    "username", "retired_username",


Remove retired_username from this place. And consider looking at other places also if exists.

Akanshu-2u · 2026-07-16T08:07:10Z

+            },
+        ),
+        (
+            "pii-django-model-bases",


For reviewers: Historical models are not considered in this PR. This will be an incremental/next phase change.

Akanshu-2u · 2026-07-16T08:16:59Z

+
+    def _test_references_flag(self, test, flag):
+        """
+        Return True if AST node *test* references the squelch *flag*.


reorganize the pattern number to reduce traversal . The most frequestly used patterns must be placed on top.

Akanshu-2u · 2026-07-16T08:31:32Z

+        """
+        Recursively inspect *node*; return the first PII term found, or None.
+        Checks Name, Attribute, f-strings, binary ops, dicts, and nested calls.
+        String literals are NOT checked.


There are a lot of ifs and elifs here, so try using switch case instead.

Akanshu-2u · 2026-07-16T08:36:14Z

+                if ancestor.name in model_bases:
+                    is_model_subclass = True
+                    break
+        except Exception:  # pylint: disable=broad-except


Check if pylint inline comment is required, if not please remove.
Check if we can log the exception raised.

Akanshu-2u · 2026-07-16T10:22:44Z

+
+# Exact identifiers that superficially match PII terms but are approved
+# as non-sensitive (surrogate keys, flag fields, system fields).
+pii-safe-key-patterns =


Remove the pii terms and just keep prefix or suffix. Ex: user_id, remove user and safe key will be _id .

vgulati-apphelix · 2026-07-16T10:37:29Z

+pii-terms =
+    email,
+    secondary_email,
+    username,
+    retired_username,
+    password,
+    full_name,
+    first_name,
+    last_name,
+    phone,
+    phone_number,
+    birth_date,
+    ip_address,
+    address,
+    mailing_address,
+    gender,
+    profile_image,
+    job_title,
+    social_link


Suggested change

pii-terms =

email,

secondary_email,

username,

retired_username,

password,

full_name,

first_name,

last_name,

phone,

phone_number,

birth_date,

ip_address,

address,

mailing_address,

gender,

profile_image,

job_title,

social_link

```suggestion : Because matching uses substring containment, several terms are redundant.

_DEFAULT_PII_TERMS = (

"email",

"username",

"password",

"full_name",

"first_name",

"last_name",

"phone",

"birth_date",

"ip_address",

"address",

"gender",

"profile_image",

"job_title",

"social_link",

)

Akanshu-2u added 8 commits July 6, 2026 10:08

fix: added squelch and no-pii annotation check

b4a3e0d

fix: fixed pylint issues

51be047

fix: shortened the annotations

177e845

fix: added high confidence pii keywords

6ce88d9

Merge branch 'master' into aaich/pii-squelch-annotation

585e403

fix: added safe keywords

4bd9ec0

fix: added some more keywords

39d703d

fix: fixed annotations

d3ef6c8

ttak-apphelix reviewed Jul 15, 2026

View reviewed changes

rahulkanneri-2u reviewed Jul 15, 2026

View reviewed changes

ttak-apphelix reviewed Jul 15, 2026

View reviewed changes

Akanshu-2u added 3 commits July 15, 2026 12:15

fix: fixed readability and remove blank spaces

a9bc81e

fix: added space formatting

307f905

fix: removed the safe function implementation

ba31f71

vgulati-apphelix reviewed Jul 15, 2026

View reviewed changes

fix: refactored the code

fcb3f1c

Akanshu-2u commented Jul 16, 2026

View reviewed changes

vgulati-apphelix reviewed Jul 16, 2026

View reviewed changes

Akanshu-2u added 4 commits July 17, 2026 10:17

fix: replaced location place and added switch case

920b3eb

fix: simplified with reduced pii-terms

30067fe

fix: fixed the pylint failure

1a5ba2d

fix: removed the safe keys for testing of false positives

cf5ecd0

Uh oh!

Conversation

Akanshu-2u commented Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description:

Solution:

Private JIRA Ticket:

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Akanshu-2u commented Jul 7, 2026 •

edited

Loading