From 14df71e4e7c262a0c82f9a4e5cc5c8366b9c2b01 Mon Sep 17 00:00:00 2001 From: Robert Sparks Date: Wed, 2 Jun 2021 15:00:55 +0000 Subject: [PATCH 1/3] Capturing changes to pre-commit and testing commit hook correction. Related to #3297. Commit ready to merge. - Legacy-Id: 19056 --- hooks/pre-commit | 217 ++++++++++++++--------------------------------- 1 file changed, 62 insertions(+), 155 deletions(-) diff --git a/hooks/pre-commit b/hooks/pre-commit index 4ae942856..a2a0fe8d0 100755 --- a/hooks/pre-commit +++ b/hooks/pre-commit @@ -1,162 +1,69 @@ -#!/usr/bin/env python +#!/bin/bash +# +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +# +# +# $Id$ +# +# Prevents some SHA-1 collisions to be commited +# Test fo the 320 byte prefix found on https://shattered.io/ +# If the files are committed in the same transaction, svnlook +# will error out itself due to the apparent corruption in the +# candidate revision -""" -An SVN pre-commit hook which requires that commits either are marked as -whitespace cleanup commits, and contain no non-whitespace changes, or -leave whitespace alone on lines without code changes. -""" +REPOS="$1" +TXN="$2" +SVNLOOK=/usr/bin/svnlook +YEAR=$(date +%Y) -import os -import sys -import difflib -#import debug -from pysvn import Client, Transaction +$SVNLOOK changed -t "$TXN" "$REPOS" +if [ $? -ne 0 ]; then + echo "svnlook failed, possible SHA-1 collision" >&2 + exit 2 +fi -prog = os.path.basename(sys.argv[0]) +FILES=$($SVNLOOK changed -t "$TXN" "$REPOS" | grep -Ev '^D ' | /usr/bin/awk '{print $2}') +for FILE in $FILES; do + if [ -f $FILE ]; then + # Check against known sha-1 collision attack. Someone committing 2 different files with this + # known hash collision could otherwise break the repository. + PREFIX=$($SVNLOOK cat -t "$TXN" "$REPOS" "$FILE" | head -c320 | /usr/bin/sha1sum | cut -c-40) + if [ "$PREFIX" = 'f92d74e3874587aaf443d1db961d4e26dde13e9c' ]; then + echo "known SHA-1 collision rejected" >&2 + exit 3 + fi -def die(msg): - sys.stderr.write("\n%s: Error: %s\n" % (prog, msg)) - sys.exit(1) + # Verify copyright year + if [[ $FILE == */ietf/*.py || -s $FILE ]]; then + $SVNLOOK cat -t "$TXN" "$REPOS" "$FILE" | head -n 3 | grep -q "Copyright .*IETF Trust .*$YEAR.*" || { + echo " + Bad or missing copyright note in $FILE. + Expected 'Copyright The IETF Trust ... $YEAR, All Rights Reserved', + (or similar) at the start of the file. -if len(sys.argv) <= 1: - die("Expected arguments: REPOSITORY TRANSACTION, found none") - -if len(sys.argv) <= 2: - die( "Expected arguments: REPOSITORY TRANSACTION, found only '%s'" % sys.argv[1]) + For bulk correction of copyright statements, try bin/check-copyright with + patching: -repo = sys.argv[1] -txname = sys.argv[2] -tx = Transaction(repo, txname) -client = Client() + \$ bin/check-copyright -p \$(svn st | cut -c 9- | grep '\.py\$' ) | patch -p0 -is_whitespace_cleanup = "whitespace cleanup" in tx.revpropget("svn:log").lower() - -def normalize(s): - return s.rstrip().expandtabs() - -def normalize_sequence(seq): - o = [] - for l in seq: - o.append(normalize(l)) - return o - -def normalize_file_end(seq): - while True and seq: - if seq[-1].strip() == "": - del seq[-1] - else: - break - return seq - -def count(gen): - return sum(1 for _ in gen) - -# Function with side effects. Acts on global varaibles -def inc_ab(flag): - global a, b - if flag == ' ': - a += 1; b += 1 - elif flag == '-': - a += 1 - elif flag == '+': - b += 1 - elif flag == '?': - pass - else: - raise ValueError("Unexpected ndiff mark: '%s' in: %s" % (flag, plain_diff[i])) - -def get_chunks(unidiff): - if not unidiff: - return [], [] - chunks = [] - chunk = [] - intro = unidiff[0:2] - for line in unidiff[2:]: - if line.startswith("@@"): - if chunk: - chunks.append(chunk) - chunk = [line] - else: - chunk.append(line) - chunks.append(chunk) - return intro, chunks - -changes = tx.changed() -issues = {} -context = 3 -for path in changes: - action, kind, mod, propmod = changes[path] - - # Don't try to diff added or deleted files, on ly changed text files - if not (mod and action == "R"): - continue - - # Don't try do diff binary files - mimetype = tx.propget("svn:mime-type", path) - if mimetype and not mimetype.startswith("text/"): - continue - - new = tx.cat(path).splitlines() - old = client.cat("file://"+os.path.join(repo,path)).splitlines() - - # Added trailing space can mess up the comparison -- eliminate it - new = normalize_file_end(new) - old = normalize_file_end(old) - - plain_diff = list(difflib.unified_diff(old, new, "%s (repository)"%path, "%s (commit)"%path, lineterm="" )) - old = normalize_sequence(old) - new = normalize_sequence(new) - white_diff = list(difflib.unified_diff(old, new, "%s (repository)"%path, "%s (commit)"%path, lineterm="")) - - plain_count = len(plain_diff) - white_count = len(white_diff) - -# for i in range(len(white_diff)): -# sys.stderr.write("%-80s | %-80s\n" % (normalize(plain_diff[i][:80]), normalize(white_diff[i][:80]))) - if white_count != plain_count and not is_whitespace_cleanup: - intro, plain_chunks = get_chunks(plain_diff) - intro, white_chunks = get_chunks(white_diff) - deletes = [] - for chunk in white_chunks: - for i in range(len(plain_chunks)): - if chunk == plain_chunks[i]: - deletes += [i] - deletes.reverse() - for i in deletes: - del plain_chunks[i] - issue = intro - for chunk in plain_chunks: - issue += chunk - if len(plain_chunks) > 1: - are = "are"; s = "s"; an = "" - else: - are = "is"; s = ""; an = "an " - issues[path] = issue - if white_count != 0 and is_whitespace_cleanup: - intro, white_chunks = get_chunks(white_diff) - if len(white_chunks) > 1: - are = "are"; s = "s"; an = "" - else: - are = "is"; s = ""; an = "an " - issues[path] = white_diff - -if issues: - if is_whitespace_cleanup: - die("It looks as if there are non-whitespace changes in\n" - "this commit, but it was marked as a whitespace cleanup commit.\n\n" - "Here %s the diff chunk%s with unexpected change%s:\n\n%s\n\n" - "Declining the commit due to a mix of code and spaces-only changes. Please\n" - "avoid mixing whitespace-only changes with code changes. See details above." % - (are, s, s, '\n\n'.join([ '\n'.join(issues[path]) for path in issues ])) - ) - - else: - die("It looks as if there are spaces-only changes in this\n" - "commit, but it was not marked as a whitespace cleanup commit.\n\n" - "Here %s the diff chunk%s with unexpected change%s:\n\n%s\n\n" - "Declining the commit due to a mix of code and spaces-only changes. Please\n" - "avoid mixing whitespace-only changes with code changes. See details above." % - (are, s, s, '\n\n'.join([ '\n'.join(issues[path]) for path in issues ])) - ) - -sys.exit(0) + " >&2 + exit 3 + } + fi + fi +done From cc3047e7197db44fb033478215b7102ea433b883 Mon Sep 17 00:00:00 2001 From: Robert Sparks Date: Wed, 2 Jun 2021 16:22:57 +0000 Subject: [PATCH 2/3] Continued testing of commit hooks. Related to #3297. Commit ready to merge. - Legacy-Id: 19057 --- hooks/trac-post-commit-hook | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/hooks/trac-post-commit-hook b/hooks/trac-post-commit-hook index fb4545a43..5514d53ac 100755 --- a/hooks/trac-post-commit-hook +++ b/hooks/trac-post-commit-hook @@ -79,7 +79,7 @@ from datetime import datetime from optparse import OptionParser, OptionGroup import syslog -syslog.openlog("post-commit-trac", syslog.LOG_PID, syslog.LOG_LOCAL0) +syslog.openlog("post-commit-trac", syslog.LOG_PID, syslog.LOG_USER) log = syslog.syslog parser = OptionParser() @@ -94,6 +94,7 @@ parser.add_option('-p', '--project', dest='project', parser.add_option('-r', '--revision', dest='rev', help='Repository revision number.') parser.add_option('-a', '--action', dest='action', default=None, help='The action to take on a ticket (default: none)') +parser.add_option('-R', '--repository-name', dest='repo_name', default=None, help='Which repository to look in, if the trac instance has multiple repositories') group = OptionGroup(parser, "DEPRECATED OPTIONS") group.add_option('-u', '--user', dest='user', @@ -123,8 +124,8 @@ from trac.versioncontrol.api import NoSuchChangeset ticket_prefix = '(?:#|(?:ticket|issue|bug)s?[: ]?#?)' ticket_reference = ticket_prefix + '[0-9]+' -action_pattern = "(?:(?i)(?:fix(?: for)?|fixes|close|closes|addresses|references|refs|re|relates to|related to|see|described in))" -ticket_command = (r'(?P%s).?(?P%s(?:(?:[, &]*|[ ]?and[ ]?)%s)*)' % (action_pattern, ticket_reference, ticket_reference)) +action_pattern = "(?:(?i)(?:fix(?: for| to)?|fixes|fixed|close|closes|completes|addresses|references|refs|re|relates to|related to|see|described in))" +ticket_command = (r'(?P%s).?(?P%s(?:(?:[, &]*|[, ]+and[ ]?)%s)*)' % (action_pattern, ticket_reference, ticket_reference)) if options.envelope: ticket_command = r'\%s%s\%s' % (options.envelope[0], ticket_command, @@ -148,9 +149,12 @@ class CommitHook: _supported_cmds = {'fix': '_cmdClose', 'fix for': '_cmdClose', + 'fix to': '_cmdClose', 'fixes': '_cmdClose', + 'fixed': '_cmdClose', 'close': '_cmdClose', 'closes': '_cmdClose', + 'completes': '_cmdClose', 'addresses': '_cmdRefs', 'described in': '_cmdRefs', @@ -165,7 +169,7 @@ class CommitHook: def __init__(self, project=options.project, author=options.user, rev=options.rev, url=options.url): self.env = open_environment(project) - repos = self.env.get_repository() + repos = self.env.get_repository(options.repo_name) repos.sync() # Instead of bothering with the encoding, we'll use unicode data From 52d274f73082278a4278c4b243e1bd681b32b606 Mon Sep 17 00:00:00 2001 From: Robert Sparks Date: Wed, 2 Jun 2021 16:24:58 +0000 Subject: [PATCH 3/3] Captured the last of the current commit hooks. Fixes #3297. Commit ready for merge. - Legacy-Id: 19058 --- hooks/post-commit | 52 +++++++++++++++++++++++++++-------------------- 1 file changed, 30 insertions(+), 22 deletions(-) diff --git a/hooks/post-commit b/hooks/post-commit index 7495ec5cf..f68726a25 100755 --- a/hooks/post-commit +++ b/hooks/post-commit @@ -44,9 +44,9 @@ # http://svn.collab.net/repos/svn/trunk/contrib/hook-scripts/ # Log everything for debug, otherwise use explicit logging (further down) -#[ "$LOGGING" ] || LOGGING=1 { exec $0 "$@" 2>&1 | logger -p local0.info -t "commit"; } +#[ "$LOGGING" ] || LOGGING=1 { exec $0 "$@" 2>&1 | logger -p user.info -t "commit"; } -logger -p local0.info -t "hook" "${0##*/} $*" +logger -p user.info -t "hook" "${0##*/} $*" repo="$1" rev="$2" @@ -56,16 +56,16 @@ program=${0##*/} progdir=${0%/*} thishost="$(/bin/hostname)" thishost="${thishost%%.*}" -svnpath="/home/svn" -trac="/www/tools.ietf.org/tools/ietfdb/" -svn_url="https://svn.tools.ietf.org/svn/tools/ietfdb" -trac_url="https://trac.tools.ietf.org/tools/ietfdb" +svnpath="/a/svn/tools/ietfdb" +trac="/a/www/www6s/trac/ietfdb/" +svn_url="https://svn.ietf.org/svn/tools/ietfdb" +trac_url="https://trac.ietf.org/trac/ietfdb" # Do a local backup -relpath=${repo#$svnpath/} -bckpath="$svnpath/.backup/$thishost/$relpath" -[ -d $bckpath ] || mkdir -p $bckpath -/usr/bin/svn-fast-backup -q $repo $bckpath +#relpath=${repo#$svnpath/} +#bckpath="$svnpath/.backup/$thishost/$relpath" +#[ -d $bckpath ] || mkdir -p $bckpath +#/usr/bin/svn-fast-backup -q $repo $bckpath # Inform trac about a new changeset trac-admin $trac changeset added ietfdb $rev @@ -75,7 +75,7 @@ comments=$(/usr/bin/svnlook log $repo -r $rev) files=$(/usr/bin/svnlook changed $repo -r $rev) dirs=$(/usr/bin/svnlook dirs-changed -r$rev $repo) -logger -p local0.info -t "commit" "dirs '$dirs'" +logger -p user.info -t "commit" "dirs '$dirs'" # Look for 'requirements.txt' above the committed change. Looking only for # filechanges, not for dirchanges, filters out commits which are just tree @@ -84,28 +84,36 @@ branch=$($progdir/svnfind --filechange --dirpath $repo $rev "requirements.txt") if [ -n "$branch" ]; then # Update trac tickets - /usr/bin/python $progdir/trac-post-commit-hook -p "$trac" -r "$rev" 2>&1 | logger -t "svn post-commit" -p "user.error" -i + /usr/bin/python2.7 $progdir/trac-post-commit-hook -p "$trac" -r "$rev" 2>&1 | logger -t "svn post-commit" -p "user.error" -i # Notify buildbot - filenames=$(/usr/bin/svnlook changed $repo -r $rev | sed -r -e 's/^ *[^ ]+ +//' -e "s|$branch/||") +# filenames=$(/usr/bin/svnlook changed $repo -r $rev | sed -r -e 's/^ *[^ ]+ +//' -e "s|$branch/||") +# # Notify local build master /usr/local/bin/buildbot sendchange \ - --master="zinfandel.tools.ietf.org:9989" --auth="ietfdb:BRiR6XcT7x3$" \ - --who="$committer" --repository="https://svn.tools.ietf.org/svn/tools/ietfdb/" \ - --vc=svn --branch="$branch" --revision=$rev --property=xproperty:xvalue \ - --revlink="https://trac.tools.ietf.org/tools/ietfdb/changeset/$rev" \ - --comments="$comments" $filenames > /dev/null +# --master="zinfandel.tools.ietf.org:9989" --auth="ietfdb:BRiR6XcT7x3$" \ +# --who="$committer" --repository="https://svn.tools.ietf.org/svn/tools/ietfdb/" \ +# --vc=svn --branch="$branch" --revision=$rev \ +# --revlink="https://trac.tools.ietf.org/tools/ietfdb/changeset/$rev" \ +# --comments="$comments" $filenames > /dev/null + # Notify remote build master (must use the remote buildbot binary to match version) +# ssh henrik@dunkelfelder.tools.ietf.org /usr/local/bin/buildbot sendchange \ +# --master="dunkelfelder.tools.ietf.org:9989" --auth="ietfdb:BRiR6XcT7x3$" \ +# --who="$committer" --repository="https://svn.tools.ietf.org/svn/tools/ietfdb/" \ +# --vc=svn --branch="$branch" --revision=$rev \ +# --revlink="https://trac.tools.ietf.org/tools/ietfdb/changeset/$rev" \ +# --comments="$comments" $filenames > /dev/null fi # Log the commit -logger -p local0.info -t "commit" "$relpath r$rev $committer" -logger -p local0.info -t "commit" "branch: $branch" +logger -p user.info -t "commit" "$relpath r$rev $committer" +logger -p user.info -t "commit" "branch: $branch" # Notify committers if [[ $comments =~ ready.(for|to).merge ]]; then - mail $(< $progdir/notify-email.txt) -s "[svnhook] Svn commit ready for merge: $relpath | $committer: ${comments:0:42}..." <<-EOF | logger -p local0.info -t "ready for merge email" + mail $(< $progdir/notify-email.txt) -s "[svnhook] Svn commit ready for merge: $relpath | $committer: ${comments:0:42}..." <<-EOF | logger -p user.info -t "ready for merge email" $committer has a commit ready for merge: $relpath/$branch [$rev]: @@ -123,7 +131,7 @@ if [[ $comments =~ ready.(for|to).merge ]]; then else - mail $(< $progdir/notify-email.txt) -s "[svnhook] Svn commit to $relpath | $committer: ${comments:0:42}..." <<-EOF | logger -p local0.info -t "commit email" + mail $(< $progdir/notify-email.txt) -s "[svnhook] Svn commit to $relpath | $committer: ${comments:0:42}..." <<-EOF | logger -p user.info -t "commit email" $committer has made a new SVN commit in $relpath/$branch [$rev]: