Repository navigation
P1-10: Sonar exclusions register for stage-eks (coordinator) #449
Description
Parent: #388 Phase: P1 Model: coordinator
Scope
Decision 2026-10-09 (Matthew): every Sonar issue on stage-eks across security, reliability and maintainability is either fixed in code with no behavior change, or covered by a documented project-level issue exclusion. This issue is the register. Each exclusion is one row: rule(s), path pattern, reason, who proposed it, date. Exclusions are applied in SonarCloud project settings (sonar.issue.ignore.multicriteria) by the coordinator only.
Register
| # | Rule | Path pattern | Reason | Proposed by |
|---|---|---|---|---|
| 1 | * |
src/olympia/migrations/** |
Historical one-time schematic SQL already applied to production; not maintained code. | coordinator, 2026-10-09 |
| 2 | * |
static/js/zamboni/*_templates.js |
Pre-compiled underscore templates (generated code; source in comments). | front-end builder, 2026-10-09 |
Done when
- Overview cards for security, reliability and maintainability on
stage-eksread 0. - Every exclusion above has a reason a reviewer can check.
Hard limits
PRD section 0. Exclusions never cover a finding that is fixable without behavior change.
Metadata
Metadata
Assignees
Type
Fields
No fields configured for issues without a type.
Activity
mzeier commentedon Oct 9, 2026
Register additions 2026-10-09 (coordinator), applied in project settings
javascript:S3504(prefer let/const)static/js/**uglify-js3.8.1 (package.json), an ES5-only parser;compress_assets._minifydiscards the exit code, so ES2015+ syntax would silently produce missing or stale-min.jsbundles. Upgrading the minifier is a separate toolchain decision (P4).javascript:S3524(arrow functions)static/js/**javascript:S3512(template literals)static/js/**javascript:S6582(optional chaining)static/js/**javascript:S6606(nullish coalescing)static/js/**javascript:S7781(String.replaceAll)static/js/**Runtime-API modernizations that are ES5-syntax-compatible (e.g.
Number.parseInt,Array#includes) stay in scope.mzeier commentedon Oct 9, 2026
Register additions 2026-10-09 (from the zamboni lane's proposals Z1 to Z10 on #420; applied in project settings)
javascript:S7740, S7760, S4138, S7780, S6666, S7751static/js/**Array#flat); bundles are minified by uglify-js 3.8.1 and ATN pages open in Thunderbird content tabs down to Gecko 60. Extends rows 3 to 8.javascript:S7761static/js/zamboni/init.jsdatasetyields undefined wheregetAttributeyields null;JSON.parse(undefined)throws on devhub/index.html, which loads init.js without those data attributes.javascript:S1874static/js/zamboni/{buttons,discovery_pane,global}.jsunescapeUTF-8 trick, feature-detectedAddSearchProvider).javascript:S7758, S3696static/js/zamboni/{files,themes_review}.jsfromCodePointdiffers fromfromCharCodeabove 0xFFFF; throwing Error objects changes uncaught-error text.javascript:S8786static/js/zamboni/{browser,files}.jsjavascript:S3776, S9381static/js/zamboni/{browser,buttons,files,l10n,validator}.jsjavascript:S2245static/js/zamboni/{discovery_video,global}.jsjavascript:S5696discovery_pane.js;S5332analytics.js;S1656files.js;S1848reviewers.js;S3516devhub.js;S3800global.jssetPosreturn value relied on by callers.Rejected: Z8 (
S1135TODO markers). A TODO note can be rewritten as a plain note referencing a tracking issue without losing context or changing behavior; the lane does that instead.mzeier commentedon Oct 9, 2026
Register additions 2026-10-09 (from the impala lane, #452 / #467; applied)
javascript:S930static/js/impala/stats/manager.jscallback; removing the argument changes a throwing path.javascript:S3776static/js/impala/stats/chart.jsjavascript:S7766static/js/impala/stats/dateutils.jsString.max/minused for lexicographic ISO-date comparison;Math.max/minwould coerce to NaN.S4138,S7760,S7740proposals are already covered by row 9 (static/js/**).mzeier commentedon Oct 9, 2026
| 20 |
css:S4649(generic font family) |static/css/**| Adding a generic family changes the computed fallback font wherever the named face is not installed; a visible change. Consistent with the #420 review classification. Applied 2026-10-09. | CSS lane / coordinator |mzeier commentedon Oct 9, 2026
| 21 |
python:S7492(prefer generator in all/any) |src/olympia/devhub/views.py| The list form forces every ModelForm.is_valid() to run in the view; the generator would short-circuit and move instance mutation and translation DB writes into template rendering. Ordering change, so kept as a list. Applied 2026-10-09. | review of #485 |Revert the S7492 all([...]) -> all(...) swaps in ownership/version_edit
mzeier commentedon Oct 9, 2026
| 22 |
Web:S6851(alt text wording) |src/olympia/**/templates/**,src/olympia/templates/**| Rewording alt text changes translatable msgids; existing de/fr translations would be dropped and every locale regresses to English. Applied 2026-10-09. | review of #490 |Revert alt-text reword; fix test broken by the main tag swap
P1-12: Sonar fixes in olympia.addons (#477)
mzeier commentedon Oct 9, 2026
Register additions 2026-10-09 (CSS lane proposals on #453; applied)
css:S8778static/css/devhub/new-landing/base.less@importdirectives resolved at compile time, not runtime CSS imports; false positive.css:S1874(obsolete tag selectors)static/css/**tt,center,acronymetc. in resets and legacy pages; changing the selector changes what is styled.css:S4670(unknown type selector)impala/base.css,impala/collections.less,restyle/restyle.lessmastheadis a real custom element used verbatim in templates; false positive.css:S4657static/css/zamboni/zamboni.cssbackground-imagePNG is a live progressive-enhancement fallback before the gradient shorthand.css:S4666(duplicated selectors)restyle/restyle.less,impala/base.css,impala/faux-zamboni.less#aux-navchild vs descendant pair, the HTML5 Boilerplate clearfix hack); merging changes the cascade. The order-independent ones in these files were fixed first.Sonar fixes in olympia.translations: dict comprehensions, parameter r…
17 remaining items
mzeier commentedon Oct 10, 2026
Rows 38-40 (JS impala lane, #452 round 3 comment, verified by coordinator): javascript:S6551 static/js/impala/stats/stats.js (queryParams is reassigned to the $.param string before concatenation; analyzer false positive), javascript:S8786 static/js/impala/global.js (no equivalent regex without the flagged shape; href inputs, no nested quantifiers), javascript:S1135 static/js/impala/persona_creation.js (TODO tracked in #464). Appended to atn-briefs/sonar-exclusions.json (153 rows).
mzeier commentedon Oct 10, 2026
Lane proposals consolidated (coordinator, 2026-10-10)
A full read of every lane comment and PR body found 62 proposed exclusions not yet in the consolidated list (
atn-briefs/harvest-exclusions.mdhas the sources and reasons verbatim). Disposition:Accepted, rows 41-71 (appended to
atn-briefs/sonar-exclusions.json, now 184 rows; reasons are the proposers' own, checked by the coordinator):python:S1172onamo/context_processors.py,lib/jingo_minify_helpers.py,files/decorators.py,amo/cron.py,amo/storage_utils.py,addons/indexers.py,users/backends.py,users/widgets.py,translations/fields.py,translations/widgets.py: signatures fixed by Django/DRF/signal/stdlib interfaces (context processorrequest,authenticate(request, ...),Widget.render(... renderer=), signalsender,gc(test_result=),walk_storage(followlinks=)).python:S107amo/utils.py(send_mailsignature is an internal API with many callers);python:S1542onamo/templatetags/jinja_helpers.py,core/logger.py,search/templatetags/jinja_helpers.pyandpython:S116onfiles/utils.py,reviewers/sql_model.py,python:S1700lib/cache.py: renames of public names are API changes, not cleanups.python:S5843onversions/compare.pyandservices/utils.py:version_redrives version parsing everywhere; restructuring risks changing which strings match.python:S6353andpython:S7510onstats/.../update_counts_from_file.py:[0-9]->\dis not equivalent under Python 3 str regex;reversed(sorted())vssorted(reverse=True)differ on ties.python:S5747api/exceptions.py: bareraiseinside DRF's handler path is correct.python:S1854addons/templatetags/jinja_helpers.py: locals consumed vianew_context(**locals()), analyzer false positive.css:S7924static/css/**(contrast ratios are the shipped design; changing colors is a visible change);css:S8759narrowed fromstatic/css/**to the three files that hold every instance (shield_study/main.css,impala/promos.less,zamboni/discovery-pane.css);css:S4648narrowed tocommon/normalize.cssandshield_study/main.css;css:S4666onzamboni/jquery-ui/**andlegacy/main.css.Held (templates lane, 21 rows:
Web:S6853x12 files,Web:S6850x2,Web:LinksIdenticalTextsDifferentTargetsCheckx2,Web:S1827,Web:S5255details.html,Web:InputWithoutLabelCheck,Web:S6819x2): the lane is re-working these in round 3 with attribute-only fixes (for/id,aria-labelledby); whatever it re-proposes after that pass is accepted. No directory-widedevhub/**/*.htmlpattern will be used.Declined, a no-behavior-change fix exists and is in a round 3 lane list:
python:S8519amo/search.py (_first_item()pattern from #473);python:S1192settings_base.py;python:S9081x2 andpython:S8997x4 in reviewers/addons tests (new=mock.Mock(...),mail.outbox.clear(),override_settings);python:S6903x2 (datetime.now(timezone.utc).replace(tzinfo=None)is a drop-in where a naive datetime is expected).Still open and not exclusion material (lane A round 3):
python:S9152x6 contextmanager cleanup,python:S112x2,python:S6559,python:S5797(if self.is_persona:on a method object, a real bug to be evaluated under stop-and-ask if fixing it changes behavior).P1-11 round 2: Sonar fixes in devhub (S7500, S9081, S3415/S5914, S907…
mzeier commentedon Oct 10, 2026
Rows 72-73 (lane A round 3, #533, confirmed by independent review): python:S9152 on src/olympia/files/utils.py (atomic_lock: releasing the lock on the exception path changes locking semantics) and src/olympia/lib/cache.py (assert_cache_requests: an assertion after yield on the error path would mask the original exception). The third S9152 site, amo/utils.py no_jinja_autoescape, is FIXED in #533 as the one allowed leak fix: a template error inside the block left autoescape disabled on the process-wide Jinja environment. List now 186 rows.
Revert stats/update_counts_from_file.py S6353/S7510 fixes: excluded r…
mzeier commentedon Oct 10, 2026
Templates lane round 3 proposals (#549 body), coordinator disposition 2026-10-10
Each row was checked by an independent reviewer against the template, CSS and JS. Accepted, rows 74-81 (list now 194 rows):
Web:S6819browse/personas/category_landing.htmlandsearch/results.html: theroleattributes carry layout CSS (.category-landing [role="main"]restyle.less:235 / zamboni.css:1620;section.secondarywidth restyle.less:1119); removing the role or swapping the tag changes layout.Web:S6850devhub/versions/list.htmlandstats/popup.html: headings are filled by JS at runtime (devhub.js:580-583, chart.js:448), so they are empty only in the static source.Web:S1827templates/captcha/widget_nocaptcha.html: verbatim Google noscript fallback markup;scrolling="no"has no CSS equivalent andframeborderedits to vendor markup are not worth the risk.Web:LinksIdenticalTextsDifferentTargetsCheckimpala/user_login.htmlandzadmin/addon_manage.html: the two links are the branches of an if/else and never render together; analyzer false positive.Web:S5255addons/impala/details.htmlline 25 only: the aside mixes ratings, usage counts and manage buttons, and no existing msgid describes it; a new translatable string is not allowed by the program rules. The second S5255 at line 183 is NOT excluded (see below).Sent back for fixing (attribute-only, no new strings, same techniques the lane used in #543-#549):
Web:S6853submit/describe.html:51,72 (for="id_slug"etc.; DescribeForm has no prefix; the Sonar duplication gate is the coordinator's problem, not a reason to exclude), submit/upload.html:56 (label id +role="group" aria-labelledbyon the existing inner div), forms_shared/media.html:24 (aria-labelledbyon<ul id="icons_default">), media.html:95 and :100 (role="group" aria-labelledbyon the existing div at line 106);Web:S5255details.html:183 (aria-label="{{ _('More about this add-on') }}", msgid exists at pot:9651).Fix always-true method-object test in Addon.tags_partitioned_by_devel…
mzeier commentedon Oct 10, 2026
Row 82 (coordinator, 2026-10-10): Web:S6819 on src/olympia/devhub/templates/**. The round 3 label-association fixes (#543, #545, #547, #555) label radio/checkbox groups with role=group or role=radiogroup plus aria-labelledby on an existing div or ul, because the fieldset/legend swap Sonar prefers is a visible rendering change (main.css:230-270 and zamboni.css:1359 restyle fieldset and legend; see the #530 review). Sonar then raises S6819 (use fieldset instead of group role) on exactly those containers. Consistent, deliberate trade; excluded for devhub templates. List now 195 rows.
R3 follow-up: Sonar S6853/S5255 attribute-only fixes graded fixable in …
mzeier commentedon Oct 10, 2026
Row 83 (coordinator, 2026-10-10): python:S6559 on src/olympia/devhub/forms.py (AuthorForm). Replacing exclude=('addon',) with an explicit fields= tuple drops the pk from ModelForm.initial and breaks the AuthorFormSet round-trip (35 failures in devhub/tests/test_views_ownership.py on #537 before the hunk was reverted). exclude= is the only equivalent form. List now 196 rows.
mzeier commentedon Oct 10, 2026
Row 84 (coordinator, 2026-10-10): python:S1172 on src/olympia/access/middleware.py. The flagged unused request parameter at line 30 is the Django middleware hook signature (process_request(self, request)); it cannot be removed. Same category as rows 41-50. List now 197 rows.
mzeier commentedon Oct 10, 2026
Rows 85-92 (coordinator, 2026-10-10, after analysis 1039ee9): Web:S6853 on the eight templates where the remaining 11 findings sit: bandwagon/ajax_new.html, bandwagon/includes/addedit.html, devhub/addons/edit/describe.html, devhub/addons/forms_shared/media.html, devhub/addons/submit/describe.html, devhub/addons/submit/upload.html, devhub/includes/macros.html, devhub/validate_addon.html. Every one of these labels heads a group of radios or checkboxes (categories, privacy, icon type, screenshots, admin override, channel, validation type), so for= cannot apply. The lane labelled each group with role=group/radiogroup plus aria-labelledby in #543, #545, #547, #549 and #555; Sonar's S6853 inspects the label element itself and still flags it, and the fieldset/legend swap it wants is a visible rendering change (main.css:230-270, zamboni.css:1359; see the #530 review). All single-control labels in these files were fixed with for=/id=. List now 205 rows.
mzeier commentedon Oct 12, 2026
Last item closed: python:S1192 in settings_base.py
PR: #565 (
449/settings-base-s1192->stage-eks)_http_url_format = 'http://%s'constant, used at all three duplicate sites (lines 229, 237, 1524). Lowercase/underscore name so Django does not expose it as a setting. No behavior change; verifiedSITE_URL,SERVICES_URL,ES_URLSevaluate identically.new_security_rating= 1/A), confirming thepython:S5332exclusion forsrc/olympia/lib/settings_base.py(register row from the 205-row list applied today) is live and covers the new-code flare this fix would otherwise trigger. This is the same fix that was reverted on450/py-b-r3-small-rules(24b31b1) on 2026-10-09 before the exclusion was applied; it now passes clean.Ready for the coordinator to merge.
mzeier commentedon Oct 12, 2026
Target met. Analysis 2026-10-12T16:13Z of stage-eks (10391c3), after Matthew applied the 205-row exclusion list and #565 merged: security 0, reliability 0, maintainability 0 (overview measures security_issues/reliability_issues/maintainability_issues all total 0; bugs 0, vulnerabilities 0, code smells 0, hotspots 0, quality gate OK). Every exclusion row and its reason is in the comments above; the list itself is atn-briefs/sonar-exclusions.json (rendered to .form, applied with apply-sonar-exclusions.sh). Closing the register; reopen if a new finding needs a documented exclusion.