eXamadeus-zz/godaddypy

delete_records may wipe all records of different type

Closed this issue · 9 comments

josteink commented

WARNING: Do not test this on a production domain. Ensure you have a backup in form of a zone-file you can reimport.

To reproduce error do the following:

  • Setup a domain on Godaddy.
  • Populate the domain with several A entries (@, www, host1, host2, etc)
  • Populate 2 TXT records (keeper, excess)
  • Use the API to attempt to delete the excess TXT-record via client.delete_records()

Observe that:

  • TXT record keeper remains.
  • excess is gone.
  • And so is all the A-records.

Oops. You just accidentally the whole DNS.

eXamadeus-zz commented

I was unable to reproduce this defect. However, during my analysis I realized that it might be possible to purge all the records if the client.get_records() call fails.

I will deliver the changes after some more testing. I will update here if anything else changes.

  • 👍1
josteink commented

I'm currently using this module in a lets encrypt validation module for Godaddy. Needless to say, until this is fixed, I won't clean up challenge records ;)

Thanks for taking time to look into this!

eXamadeus-zz commented

BTW, I have added some important documentation for that method. I really should have had it there when I first wrote it...

def delete_records(self, domain, name, record_type=None):
        """Deletes records by name.  You can also add a record type, which will only delete records with the
        specified type/name combo.  If no record type is specified, ALL records that have a matching name will be
        deleted.

        This is haphazard functionality.   I DO NOT recommend using this in Production code, as your entire DNS record
        set could be deleted, depending on the fickleness of GoDaddy.  Unfortunately, they do not expose a proper
        "delete record" call, so there isn't much one can do here...

        :param domain: the domain to delete records from
        :param name: the name of records to remove
        :param record_type: the type of records to remove

        :return: True if no exceptions occurred
        """
eXamadeus-zz commented

It's no problem. Unfortunately, this is a hackish way to delete the records. I wish GoDaddy provided a cleaner solution, but alas, we are at their mercy.

josteink commented

Thanks for the heads up. I completely understand. Life, lemons, and all that. Been there, done that :)

josteink commented

Was this fixed in eXamadeus/godaddypy@278fb30 ?

If so, this issue should probably be closed :)

eXamadeus-zz commented

Yes, but I haven't done proper testing for it yet.

Sorry for being slow! Work is busy ;(

Sent from my iPhone

On Aug 15, 2016, at 4:19 PM, Jostein Kjønigsen notifications@github.com wrote:

Was this fixed in 278fb30 ?

If so, this issue should probably be closed :)

—
You are receiving this because you were assigned.
Reply to this email directly, view it on GitHub, or mute the thread.

josteink commented

No worries. Just thought I'd help clear out stale issues. It's easy to forget to close things.

eXamadeus-zz commented

Fix added in eXamadeus/godaddypy@bd84f31. Closing this issue.

Thanks @josteink