-
Notifications
You must be signed in to change notification settings - Fork 34
Stricter filename validations #2374
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.
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
Conversation
Enforces stricter filename validation to ensure consistency across documentation files. Filenames must now: - Be lowercase - Start with an alphanumeric character or underscore - Contain only alphanumeric characters, spaces, dashes, dots, and underscores This PR adds validation for both the full file path and the filename itself, catching issues like uppercase letters in directory names and invalid starting characters. - Added `FilePathRegex` to validate the full path (lowercase, valid characters) - Added `FileNameRegex` to validate the filename starts with `[a-z0-9_]` - Combined both validations in `IsValidFileName` - Extended test coverage with 140 theory-based test cases - [x] All `OutputDirectoryTests` pass (140 tests) - [x] Validation correctly rejects uppercase in paths and filenames - [x] Validation correctly rejects filenames starting with hyphens, dots, or spaces The following files have leading underscores and need to be renamed before we can make the check strictly alphanumeric (removing underscore from allowed starting characters): - `docs/reference/elasticsearch-plugins/_reimplementing_and_extending_the_analyzers.md` - `docs/reference/elasticsearch-plugins/_other_command_line_parameters.md` - `docs/reference/elasticsearch-plugins/_plugins_directory.md` - `docs/reference/elasticsearch-plugins/_reimplementing_and_extending_the_analyzers_2.md` - `docs/extend/_publish_an_integration.md` - `deploy-manage/tools/cross-cluster-replication/_failback_when_clustera_comes_back.md` - `deploy-manage/tools/cross-cluster-replication/_failback_when_clustera_comes_back_2.md` - `deploy-manage/tools/cross-cluster-replication/_prerequisites_14.md` - `deploy-manage/tools/cross-cluster-replication/_configure_privileges_for_cross_cluster_replication_2.md` - `deploy-manage/tools/cross-cluster-replication/_perform_update_or_delete_by_query.md` - `deploy-manage/tools/cross-cluster-replication/_failover_when_clustera_is_down.md` - `deploy-manage/tools/cross-cluster-replication/_failover_when_clustera_is_down_2.md` - `deploy-manage/tools/cross-cluster-replication/_connect_to_a_remote_cluster.md` - `troubleshoot/observability/apm/_collection_of_diagnostic_information.md` - `troubleshoot/observability/apm/_agent_is_not_instrumenting_code.md` - `reference/fleet/_agent_configuration_encryption.md` - `docs/release-notes/_snippets/9.2.0+build202510300150/deprecations.md` - `docs/release-notes/_snippets/9.2.0+build202510300150/breaking-changes.md` - `docs/release-notes/_snippets/9.2.0+build202510300150/index.md` - `docs/reference/metricbeat/_host_setup.md` - `docs/reference/metricbeat/_galera_status_metricset.md` - `docs/reference/metricbeat/_live_reloading.md` - `docs/reference/metricbeat/_metricsets_70.md` - `docs/reference/filebeat/_live_reloading.md` - `docs/reference/filebeat/_set_up_the_oauth_app_in_the_salesforce_2.md` - `docs/extend/_migrating_dashboards_from_kibana_5_x_to_6_x.md` - `docs/reference/_enrichers_2.md` - `docs/reference/_usage.md` - `docs/reference/_extending_ecsdocument.md` - `docs/reference/_formatters.md` - `docs/reference/_elasticsearch_security.md` - `docs/reference/_structured_logging_with_log4j2.md` - `docs/reference/_getting_started_with_the_api.md` - `docs/reference/_options_on_elasticsearchclientsettings.md`
theletterf
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would a doc sweep be a requirement before we merge this?
Thinking of more scenarios such as unique anchors, etc.
|
We should do a pass but we don't have to block merging this, we have allow listed the offenders for now. |
| _ when strToCheck.EndsWith(".png") => true, | ||
| _ when strToCheck.EndsWith(".png") => true, | ||
| "reference/security/prebuilt-rules/audit_policies/windows/README.md" => true, | ||
| "extend/integrations/developer-workflow-fleet-UI.md" => true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if directly related to this change, but docs builds in the integrations repository are failing since around the time it was merged. With the following error:
Error: File name extend/developer-workflow-fleet-UI.md is not valid needs to be lowercase and contain only alphanumeric characters, spaces, dashes, dots and underscores
First failed build is this one: https://github.com/elastic/integrations/actions/runs/20336771987/job/58425304911
Should this path be modified like this?
| "extend/integrations/developer-workflow-fleet-UI.md" => true, | |
| "extend/developer-workflow-fleet-UI.md" => true, |
cc @elastic/ecosystem @elastic/docs-engineering
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Summary
Enforces stricter filename validation to ensure consistency across documentation files. Filenames must now:
This PR adds validation for both the full file path and the filename itself, catching issues like uppercase letters in directory names and invalid starting characters.
Changes
FilePathRegexto validate the full path (lowercase, valid characters)FileNameRegexto validate the filename starts with[a-z0-9_]IsValidFileNameTest plan
OutputDirectoryTestspass (140 tests)Files requiring updates in other repositories
The following files have leading underscores and need to be renamed before we can make the check strictly alphanumeric (removing underscore from allowed starting characters):
elasticsearch
docs/reference/elasticsearch-plugins/_reimplementing_and_extending_the_analyzers.mddocs/reference/elasticsearch-plugins/_other_command_line_parameters.mddocs/reference/elasticsearch-plugins/_plugins_directory.mddocs/reference/elasticsearch-plugins/_reimplementing_and_extending_the_analyzers_2.mdintegrations
docs/extend/_publish_an_integration.mddocs-content
deploy-manage/tools/cross-cluster-replication/_failback_when_clustera_comes_back.mddeploy-manage/tools/cross-cluster-replication/_failback_when_clustera_comes_back_2.mddeploy-manage/tools/cross-cluster-replication/_prerequisites_14.mddeploy-manage/tools/cross-cluster-replication/_configure_privileges_for_cross_cluster_replication_2.mddeploy-manage/tools/cross-cluster-replication/_perform_update_or_delete_by_query.mddeploy-manage/tools/cross-cluster-replication/_failover_when_clustera_is_down.mddeploy-manage/tools/cross-cluster-replication/_failover_when_clustera_is_down_2.mddeploy-manage/tools/cross-cluster-replication/_connect_to_a_remote_cluster.mdtroubleshoot/observability/apm/_collection_of_diagnostic_information.mdtroubleshoot/observability/apm/_agent_is_not_instrumenting_code.mdreference/fleet/_agent_configuration_encryption.mdbeats
docs/reference/metricbeat/_host_setup.mddocs/reference/metricbeat/_galera_status_metricset.mddocs/reference/metricbeat/_live_reloading.mddocs/reference/metricbeat/_metricsets_70.mddocs/reference/filebeat/_live_reloading.mddocs/reference/filebeat/_set_up_the_oauth_app_in_the_salesforce_2.mddocs/extend/_migrating_dashboards_from_kibana_5_x_to_6_x.mdecs-dotnet
docs/reference/_enrichers_2.mddocs/reference/_usage.mddocs/reference/_extending_ecsdocument.mddocs/reference/_formatters.mddocs/reference/_elasticsearch_security.mdecs-logging-java
docs/reference/_structured_logging_with_log4j2.mdgo-elasticsearch
docs/reference/_getting_started_with_the_api.mdelasticsearch-net
docs/reference/_options_on_elasticsearchclientsettings.md